transform sgen_get_descriptor to parallel safe version in job_major_mod_union_preclean #63293
Conversation
…od_union_preclean Fixes mono/mono#21369 job_major_mod_union_preclean can race with the tarjan bridge implementation that changes the vtable pointer by settings the three lower bits. this results in invalid loading of the vtable (shifted by 7 bytes) which in turn give a wrong desc to the scan functions This change is released under the MIT license.
monojenkins
requested review from
BrzVlad,
lambdageek and
naricc
as code owners
ghost
added
area-GC-mono
mono-mirror
community-contribution
|
Tagging subscribers to this area: @BrzVlad Issue Details!! This PR is a copy of mono/mono#21384, please do not edit or review it in this repo !! Fixes mono/mono#21369 job_major_mod_union_preclean can race with the tarjan bridge This change is released under the MIT license.
|
|
And we will want this on the 6.0-maui branch, too, I think, @steveisok |
|
/backport to release/6.0-maui |
|
Started backporting to release/6.0-maui: https://github.com/dotnet/runtime/actions/runs/1650358310 |
|
the release/6.0 branch maybe hit these problem? |
Yes. But it's only a problem on configurations which use the Mono GC Bridge and where the GC Bridge callback is manipulating the vtable pointer bits - which is what Xamarin.Android does. No other supported 6.0 configuration uses the GC bridge this way. The backport to release/6.0-maui will be picked up by Xamarin.Android |
the tagging is not happening in the bridge callback("user bridge callback"), rather in bridge tarjan implementation (the part that computes the scc graph) that is in the mono codebase (mono/metadata/sgen-tarjan-bridge.c) and that is called before the cross_references callback. So this could theorically happen without xamarin-android if you use the tarjan bridge implementation. The bit manipulation is not in the c++ implementation of bridge callback in xamarin/xamarin-android. ( I dont know if there is other than xamarin-android that really use the bridge, because the comments in the code seems to indicate it was created with xamarin android in mind ...) see this snippet, the tarjan bridge implementation uses this mechanism to store metadata about the nodes(gc objects) while new and old implementation use a hash_table (the gc object pointer being the key ) to retrieve per node metadata, hence not modifying the objects themselves. |
|
Is there any chance this also affects Mono WebAssembly (Blazor)? See #62054 |
|
@hakenr No. This change is only a fix for android targets. |
ghost
locked as resolved and limited conversation to collaborators
!! This PR is a copy of mono/mono#21384, please do not edit or review it in this repo !!
Do not automatically approve this PR:
* Consider how the changes affect configurations in this repo,
* Check effects on files that are not mirrored,
* Identify test cases that may be needed in this repo.
!! Merge the PR only after the original PR is merged !!
Fixes mono/mono#21369
Related to dotnet/android#6546
job_major_mod_union_preclean can race with the tarjan bridge
implementation that changes the vtable pointer by settings the three
lower bits. this results in invalid loading of the vtable
(shifted by 7 bytes) which in turn give a wrong desc to the scan
functions
This change is released under the MIT license.