Skip to content

Android: unregister automation peers when their control leaves the visual tree - #22024

Merged
MrJul merged 1 commit into
AvaloniaUI:mainfrom
ronnycohen:fix/android-a11y-peer-registry-leak
Aug 24, 2026
Merged

Android: unregister automation peers when their control leaves the visual tree#22024
MrJul merged 1 commit into
AvaloniaUI:mainfrom
ronnycohen:fix/android-a11y-peer-registry-leak

Conversation

@ronnycohen

Copy link
Copy Markdown
Contributor

What does the pull request do?

AvaloniaAccessHelper registers every AutomationPeer it hands to Android in three dictionaries — _peers, _peerIds, _peerNodeInfoProviders — and never removes any of them. The file contains no Remove and no Clear.

Since ControlAutomationPeer holds a strong reference to its Owner, any control that accessibility has explored is pinned for the lifetime of the AvaloniaView, together with the visual tree hanging off it.

This PR unregisters a peer when its control is detached from the visual tree, and unsubscribes the two peer event handlers at the same time (they were never removed either).

What is the current behavior?

Unbounded managed-memory growth on any Android app that rebuilds its visual tree, whenever an accessibility service is active on the device.

Measured on a digital-signage device that rebuilds its screen every 20–40 s (Android 13, Avalonia 12.1.1):

  • ~2.5 MB of live managed heap retained per rebuild, measured after a forced blocking gen2 collection followed by WaitForPendingFinalizers — so this is genuinely reachable memory, not uncollected garbage;
  • instance counts of the screen's widgets grew by one full screen per rebuild and never came down;
  • rooting chain, obtained with an in-process root tracer on the device:
MainActivity.Current -> AvaloniaActivity._view -> AvaloniaView._accessHelper
  -> AvaloniaAccessHelper._peers -> ControlAutomationPeer.Owner
  -> the dead control, and the rest of the dead screen through it

Left alone, it ran until the low-memory killer terminated the process.

What is the updated/expected behavior with this PR?

Stale registrations are dropped as their controls leave the visual tree. On the same bench, the retained-instance count goes from unbounded growth to a flat plateau held over 28 consecutive rebuilds, with live heap steady at ~20 MB.

Notes

Virtual view ID allocation had to change. IDs were derived from _peerNodeInfoProviders.Count, which is only unique while nothing is ever removed. With removal, Count can fall back onto an ID that is still in use, and the next registration would throw from Dictionary.Add — inside an accessibility callback. IDs now come from a monotonic counter. I hit exactly this while validating an out-of-tree version of the fix, so it is a real hazard rather than a theoretical one.

The root peer (ID 0) is deliberately left registered. GetVirtualViewAt and GetVisibleVirtualViews index _peers[0] directly, so removing it would throw. It is a single entry, owned by the view, and dies with the helper.

Re-attachment is handled by the existing code path: a control that comes back is simply registered again, receiving a fresh ID.

Checklist

  • Added unit tests? — no; AvaloniaAccessHelper is internal to Avalonia.Android and its behaviour is driven by ExploreByTouchHelper callbacks from the Android framework, so there is no existing harness to hook into. I am happy to add one if you can point me at the shape you would want.
  • Built and validated on device (Android 13, arm64).

Fixed issues

None filed — found while investigating a memory leak in a production Avalonia Android app.

…sual tree

AvaloniaAccessHelper keeps every automation peer it has ever handed to
Android in three dictionaries (_peers, _peerIds, _peerNodeInfoProviders)
and never removes any of them - the file contains no Remove or Clear call.
Each ControlAutomationPeer holds a strong reference to its Owner, so once
accessibility has explored a control, that control (and the visual tree
hanging off it) is pinned for the lifetime of the AvaloniaView.

This is invisible in an app with a static UI, but it is unbounded in one
that rebuilds its visual tree: on a digital-signage device rebuilding its
screen every 20-40 s, this retained one full dead screen per rebuild -
about 2.5 MB of live managed heap each time, measured after a forced
gen2 collection, growing until the low-memory killer stepped in.
Removing the stale entries brought the same bench from unbounded growth
to a flat plateau over 28 consecutive rebuilds.

The registration is now dropped when the peer's control is detached from
the visual tree, and the two peer event handlers are unsubscribed at the
same time (they were never removed either).

Virtual view IDs are now allocated from a monotonic counter instead of
being derived from _peerNodeInfoProviders.Count. That was safe only while
nothing was ever removed: with removal, Count can fall back onto an ID
that is still in use and the next registration would throw from
Dictionary.Add inside an accessibility callback.

The root peer (ID 0) is deliberately left registered: GetVirtualViewAt and
GetVisibleVirtualViews index _peers[0] directly, so removing it would
throw. It is a single entry, owned by the view, and dies with the helper.
@avaloniaui-bot

Copy link
Copy Markdown

You can test this PR using the following package version. 12.2.999-cibuild0068571-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added bug os-android backport-candidate-12.1.x Consider this PR for backporting to 12.1 branch labels Aug 20, 2026
@MrJul
MrJul added this pull request to the merge queue Aug 24, 2026
Merged via the queue into AvaloniaUI:main with commit 7c4123f Aug 24, 2026
11 checks passed
ronnycohen added a commit to ronnycohen/Avalonia that referenced this pull request Sep 1, 2026
… attribution

Review feedback: a single space only satisfies the platform contract, it does not
describe anything, and it hides an unnamed control instead of surfacing it. The
fallback is available - GetClassNameCore() is abstract and ControlAutomationPeer
returns Owner.GetType().Name - so a peer always has a type name, which is also what
nodeInfo.ClassName already carries.

This also corrects a claim made in the first revision of this change. The androidx
guard does not use TextUtils.isEmpty: in androidx.customview 1.1.0 and 1.2.0,
createNodeForChild tests getText() and getContentDescription() for null, strictly,
and the class contains no TextUtils reference at all (the isEmpty guard sits on the
event path, which throws populateEventForVirtualViewId()).

Since AutomationPeer.GetName() collapses a missing name to string.Empty, a node
built for an unnamed container was therefore never rejected. Only the stale virtual
view id path - reachable since AvaloniaUI#22024 unregisters peers on detach - produced a node
the platform refuses. The placeholder there is now string.Empty, and the type name
default is presented for what it is: an accessibility improvement, not a crash fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ronnycohen

Copy link
Copy Markdown
Contributor Author

Could this get its 12.1.x backport? The backport-candidate-12.1.x label is still on, and no
backport PR seems to have been opened.

One thing worth flagging before it is: this change made an existing hole in
OnPopulateNodeForVirtualView reachable. That method used to return without touching the node
when the virtual view id was not in _peers, which could not happen before — nothing was ever
removed. Now that peers are unregistered on detach, the platform can ask for a node it cached
earlier, the lookup fails, and ExploreByTouchHelper.createNodeForChild rejects the unpopulated
node it gets back: it throws from inside an accessibility callback, so the application goes down.

#22122 fixes that. If this one is backported, that one should go in the same 12.1.x, otherwise the
backport ships the new crash path without its guard.

@MrJul

MrJul commented Sep 1, 2026

Copy link
Copy Markdown
Member

The backport-candidate- label means the PR will be automatically backported to the next 12.1.x version, no further action is necessary from your part.

Also, I'm a human, can I please speak with a human in return? LLM speak is painful enough when talking to a LLM on purpose, I don't need to see it in every single comment.

MrJul pushed a commit to MrJul/Avalonia that referenced this pull request Sep 2, 2026
…UI#22122)

* Android: never leave an accessibility node without text or content description

`ExploreByTouchHelper.createNodeForChild` validates every virtual view the
callback produces and throws

    Callbacks must add text or a content description in populateNodeForVirtualViewId()

when the node carries neither. It throws from inside an accessibility callback,
so the exception is not recoverable by the application: the process goes down.

`TextUtils.isEmpty` treats `""` as empty, so an empty string does not satisfy
the contract - only a non-empty value does. Two paths in
`OnPopulateNodeForVirtualView` could produce such a node:

* **A live peer with no accessible name.** `nodeInfo.Text ??= peer.GetName()`
  looks like it only assigns when nothing was set, but `AutomationPeer.GetName()`
  and `GetHelpText()` never return null - they collapse a missing value to
  `string.Empty` - so both assignments always run and can both assign `""`.
  This happens for any peer that is a pure container: a `Panel`, a `Border`, or
  the `TextSelectorLayer` that is added when a text selection starts. On a device
  with an accessibility service running, the first touch on a text field was
  enough to bring the application down.

* **A stale virtual view id.** Since AvaloniaUI#22024 peers are unregistered when their
  control leaves the visual tree, and the platform can still ask for a node it
  obtained earlier - it caches them, and it re-queries the accessibility focused
  one. The lookup then fails and the node was left untouched, which the same
  validation rejects.

Both are fixed by guaranteeing a non-empty content description. A single space
is used deliberately: it satisfies the platform contract without inventing a
label that screen readers would announce.

Note that `AutomationPeer.GetClassName()` has the same `?? string.Empty` shape,
so it cannot serve as the fallback.

### Testing

`Avalonia.Android` builds clean. The crash paths need a device with an
accessibility service enabled and are not reachable from the unit test projects;
the fix was verified on hardware (Android 13 kiosk) where the first touch on a
text field used to take the application down on every armed process.

* Android: default an unlabelled node to its type name, and correct the attribution

Review feedback: a single space only satisfies the platform contract, it does not
describe anything, and it hides an unnamed control instead of surfacing it. The
fallback is available - GetClassNameCore() is abstract and ControlAutomationPeer
returns Owner.GetType().Name - so a peer always has a type name, which is also what
nodeInfo.ClassName already carries.

This also corrects a claim made in the first revision of this change. The androidx
guard does not use TextUtils.isEmpty: in androidx.customview 1.1.0 and 1.2.0,
createNodeForChild tests getText() and getContentDescription() for null, strictly,
and the class contains no TextUtils reference at all (the isEmpty guard sits on the
event path, which throws populateEventForVirtualViewId()).

Since AutomationPeer.GetName() collapses a missing name to string.Empty, a node
built for an unnamed container was therefore never rejected. Only the stale virtual
view id path - reachable since AvaloniaUI#22024 unregisters peers on detach - produced a node
the platform refuses. The placeholder there is now string.Empty, and the type name
default is presented for what it is: an accessibility improvement, not a crash fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Cut the explanatory comments down, per review on AvaloniaUI#22123

The rationale belongs in the pull request, not in the source. Only the two lines a
reader cannot infer from the code are kept.

* Drop the type name default, as requested in review

Keeps this PR to the crash fix alone. The default label for an unnamed node is a
separate discussion.

---------

Co-authored-by: ronnycohen <19652995+ronnycohen@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
MrJul pushed a commit to MrJul/Avalonia that referenced this pull request Sep 2, 2026
…sual tree (AvaloniaUI#22024)

AvaloniaAccessHelper keeps every automation peer it has ever handed to
Android in three dictionaries (_peers, _peerIds, _peerNodeInfoProviders)
and never removes any of them - the file contains no Remove or Clear call.
Each ControlAutomationPeer holds a strong reference to its Owner, so once
accessibility has explored a control, that control (and the visual tree
hanging off it) is pinned for the lifetime of the AvaloniaView.

This is invisible in an app with a static UI, but it is unbounded in one
that rebuilds its visual tree: on a digital-signage device rebuilding its
screen every 20-40 s, this retained one full dead screen per rebuild -
about 2.5 MB of live managed heap each time, measured after a forced
gen2 collection, growing until the low-memory killer stepped in.
Removing the stale entries brought the same bench from unbounded growth
to a flat plateau over 28 consecutive rebuilds.

The registration is now dropped when the peer's control is detached from
the visual tree, and the two peer event handlers are unsubscribed at the
same time (they were never removed either).

Virtual view IDs are now allocated from a monotonic counter instead of
being derived from _peerNodeInfoProviders.Count. That was safe only while
nothing was ever removed: with removal, Count can fall back onto an ID
that is still in use and the next registration would throw from
Dictionary.Add inside an accessibility callback.

The root peer (ID 0) is deliberately left registered: GetVirtualViewAt and
GetVisibleVirtualViews index _peers[0] directly, so removing it would
throw. It is a single entry, owned by the view, and dies with the helper.
MrJul pushed a commit to MrJul/Avalonia that referenced this pull request Sep 2, 2026
…UI#22122)

* Android: never leave an accessibility node without text or content description

`ExploreByTouchHelper.createNodeForChild` validates every virtual view the
callback produces and throws

    Callbacks must add text or a content description in populateNodeForVirtualViewId()

when the node carries neither. It throws from inside an accessibility callback,
so the exception is not recoverable by the application: the process goes down.

`TextUtils.isEmpty` treats `""` as empty, so an empty string does not satisfy
the contract - only a non-empty value does. Two paths in
`OnPopulateNodeForVirtualView` could produce such a node:

* **A live peer with no accessible name.** `nodeInfo.Text ??= peer.GetName()`
  looks like it only assigns when nothing was set, but `AutomationPeer.GetName()`
  and `GetHelpText()` never return null - they collapse a missing value to
  `string.Empty` - so both assignments always run and can both assign `""`.
  This happens for any peer that is a pure container: a `Panel`, a `Border`, or
  the `TextSelectorLayer` that is added when a text selection starts. On a device
  with an accessibility service running, the first touch on a text field was
  enough to bring the application down.

* **A stale virtual view id.** Since AvaloniaUI#22024 peers are unregistered when their
  control leaves the visual tree, and the platform can still ask for a node it
  obtained earlier - it caches them, and it re-queries the accessibility focused
  one. The lookup then fails and the node was left untouched, which the same
  validation rejects.

Both are fixed by guaranteeing a non-empty content description. A single space
is used deliberately: it satisfies the platform contract without inventing a
label that screen readers would announce.

Note that `AutomationPeer.GetClassName()` has the same `?? string.Empty` shape,
so it cannot serve as the fallback.

### Testing

`Avalonia.Android` builds clean. The crash paths need a device with an
accessibility service enabled and are not reachable from the unit test projects;
the fix was verified on hardware (Android 13 kiosk) where the first touch on a
text field used to take the application down on every armed process.

* Android: default an unlabelled node to its type name, and correct the attribution

Review feedback: a single space only satisfies the platform contract, it does not
describe anything, and it hides an unnamed control instead of surfacing it. The
fallback is available - GetClassNameCore() is abstract and ControlAutomationPeer
returns Owner.GetType().Name - so a peer always has a type name, which is also what
nodeInfo.ClassName already carries.

This also corrects a claim made in the first revision of this change. The androidx
guard does not use TextUtils.isEmpty: in androidx.customview 1.1.0 and 1.2.0,
createNodeForChild tests getText() and getContentDescription() for null, strictly,
and the class contains no TextUtils reference at all (the isEmpty guard sits on the
event path, which throws populateEventForVirtualViewId()).

Since AutomationPeer.GetName() collapses a missing name to string.Empty, a node
built for an unnamed container was therefore never rejected. Only the stale virtual
view id path - reachable since AvaloniaUI#22024 unregisters peers on detach - produced a node
the platform refuses. The placeholder there is now string.Empty, and the type name
default is presented for what it is: an accessibility improvement, not a crash fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Cut the explanatory comments down, per review on AvaloniaUI#22123

The rationale belongs in the pull request, not in the source. Only the two lines a
reader cannot infer from the code are kept.

* Drop the type name default, as requested in review

Keeps this PR to the crash fix alone. The default label for an unnamed node is a
separate discussion.

---------

Co-authored-by: ronnycohen <19652995+ronnycohen@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@MrJul MrJul added backported-12.1.x and removed backport-candidate-12.1.x Consider this PR for backporting to 12.1 branch labels Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants