Skip to content

[release/10.0.1xx] [native] Fix race publishing assembly store runtime data - #13261

Closed
filipnavara wants to merge 1 commit into
dotnet:release/10.0.1xxfrom
filipnavara:dev/fix-assembly-store-race-10
Closed

filipnavara wants to merge 1 commit into
dotnet:release/10.0.1xxfrom
filipnavara:dev/fix-assembly-store-race-10

Conversation

@filipnavara

@filipnavara filipnavara commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Fixes: #13194

Context: #13065

When two threads load the same assembly from the assembly store for the
first time, both can enter the initialization block in
EmbeddedAssemblies::assembly_store_open_from_bundles() (MonoVM) and
AssemblyStore::open_assembly() (CoreCLR). That block wrote
image_data, descriptor and debug_info_data with plain stores, and
other threads checked image_data != nullptr with a plain load to decide
whether the entry was ready.

On weakly ordered CPUs such as arm64, those stores can become visible to
another core in any order, and the reader's later loads aren't ordered
after its image_data check. A second thread can therefore see
image_data set, skip initialization, and read descriptor->data_size
while descriptor is still NULL:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x8
libmonodroid.so EmbeddedAssemblies::open_from_bundles(...)

The issue reports this from the field (about 3,200 users on
.NET for Android 36.1.69, Release, arm64). It was also reproduced
deterministically by stopping a thread under a debugger at the point
where it can see image_data set but not descriptor.

Fix: compute image_data, debug_info_data and the descriptor
(&store_entry) locally from the assembly store entry, and have the
loader use only those local values. It no longer reads them back from
the shared AssemblyStoreSingleAssemblyRuntimeData, so it can't race with
another thread that is still initializing the same entry. The shared
fields are still filled in: descriptor and debug_info_data with
relaxed atomic stores, then image_data last with __ATOMIC_RELEASE.
The readiness check reads image_data with __ATOMIC_ACQUIRE. In the
arm64 release build the check compiles to ldar and the publish to
stlr.

To support this, the MonoVM get_assembly_data() overload that took
the runtime data struct is removed (the existing pointer-and-size
overload is used instead). The CoreCLR get_assembly_data() now takes
the image pointer and size directly.

main isn't affected: the MonoVM host was removed in #12868, and #12972
changed CoreCLR to fill in this data while the assembly store is set up,
before any lookups can run.

This doesn't address #13065, which is a separate race in
map_runtime_file() on the individual-assembly (non-store) path.

There's no automated test: the race window is a few instructions wide
and depends on timing.

@filipnavara
filipnavara marked this pull request as ready for review October 26, 2026 14:03
Copilot AI balanced review requested due to automatic review settings October 26, 2026 14:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Both loaders still have plain field reads that can race with atomic writes from another in-flight initializer.

1 open issue
What changed in this PR

This release-branch fix addresses assembly-store initialization races in MonoVM and CoreCLR reported in #13194. The separate non-store race in #13065 remains outside scope.

Changes:

  • Store descriptor and debug-information pointers before publishing image_data.
  • Use release/acquire operations to order publication and readiness checks in both loaders.
File Description
src/​native/​mono/​monodroid/​embedded-assemblies.cc Adds ordered publication to MonoVM assembly-store initialization.
src/​native/​clr/​host/​assembly-store.cc Applies the same publication ordering to CoreCLR.

Review effort: Balanced

Fixes: dotnet#13194

When two threads load the same assembly from the assembly store for the
first time, both could enter the initialization block in
`open_from_bundles` (Mono) / `AssemblyStore::open_assembly` (CoreCLR).
The fields were written with plain stores, which on weakly ordered CPUs
(e.g. arm64) can become visible to other threads in any order. A thread
could therefore observe `image_data != nullptr`, skip initialization and
dereference a still-NULL `descriptor`, crashing with SIGSEGV at fault
address 0x8.

Compute `image_data`, `debug_info_data` and the descriptor locally from
the assembly store entry and use only those local values after the
readiness check, so the loaders never read the shared fields back while
another thread may still be writing them. The shared fields are written
atomically, with `image_data` published last using release semantics and
checked with acquire semantics.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The revised loaders eliminate the shared-field reads responsible for the reported race, with no unresolved blocking findings.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@filipnavara

Copy link
Copy Markdown
Member Author

Closing in favor of #13264 approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants