-
Notifications
You must be signed in to change notification settings - Fork 553
[nativeaot] fix default SynchronizationContext
#9883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Running `Mono.Android-Tests` under NativeAOT has the test failure: 03-04 23:06:20.021 6191 6211 I NUnit : SynchronizationContext_Is_ThreadingSynchronizationContextCurrent 03-04 23:06:20.032 6191 6211 E NUnit : [FAIL] 03-04 23:06:20.032 6191 6211 E NUnit : : Expected: True 03-04 23:06:20.032 6191 6211 E NUnit : But was: False 03-04 23:06:20.032 6191 6211 E NUnit : at libMono.Android.NET-Tests!<BaseAddress>+0x1482f63 03-04 23:06:20.032 6191 6211 E NUnit : at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) + 0xf3 Initially, if we called `JNIEnvInit.SetSynchronizationContext()` at startup, this can result in the static ctor for `Android.App.Application` running *too soon* before `JNIEnvInit.InitializeJniRuntime()` has completed. To fix this: * Add an empty `static ctor` to `Android.App.Application` to remove the `beforefieldinit` flag from the type. * Call `JNIEnvInit.SetSynchronizationContext()` at startup for NativeAOT. With this change in place, no crashes occur at startup and the test passes.
I've tested this locally and it doesn't seem to fix my test case. |
I think @dellis1972's issue might be something different:
|
Context: https://github.com/dotnet/android/pull/9846
Context: b6c22f3909741aa0418b2c17eef7bf3895a797bb
Running `Mono.Android-Tests` under NativeAOT has the test failure:
I NUnit : SynchronizationContext_Is_ThreadingSynchronizationContextCurrent
E NUnit : [FAIL]
E NUnit : : Expected: True
E NUnit : But was: False
E NUnit : at libMono.Android.NET-Tests!<BaseAddress>+0x1482f63
E NUnit : at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) + 0xf3
`SynchronizationContext_Is_ThreadingSynchronizationContextCurrent()`
failed because we never called `JNIEnvInit.SetSynchronizationContext()`.
Call `JNIEnvInit.SetSynchronizationContext()`, and…
--------- beginning of crash
03-07 15:48:39.566 17058 17119 F libc : Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 17119 (Thread-11), pid 17058 (droid.NET_Tests)
03-07 15:48:39.661 17145 17145 I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstoneProto
03-07 15:48:39.662 693 693 I tombstoned: received crash request for pid 17119
03-07 15:48:39.663 17145 17145 I crash_dump64: performing dump of process 17058 (target tid = 17119)
03-07 15:48:39.828 17145 17145 F DEBUG : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
03-07 15:48:39.828 17145 17145 F DEBUG : Build fingerprint: 'google/panther/panther:14/AP2A.240905.003/12231197:user/release-keys'
03-07 15:48:39.828 17145 17145 F DEBUG : Revision: 'MP1.0'
03-07 15:48:39.828 17145 17145 F DEBUG : ABI: 'arm64'
03-07 15:48:39.828 17145 17145 F DEBUG : Timestamp: 2025-03-07 15:48:39.677241696-0600
03-07 15:48:39.828 17145 17145 F DEBUG : Process uptime: 2s
03-07 15:48:39.828 17145 17145 F DEBUG : Cmdline: Mono.Android.NET_Tests
03-07 15:48:39.828 17145 17145 F DEBUG : pid: 17058, tid: 17119, name: Thread-11 >>> Mono.Android.NET_Tests <<<
03-07 15:48:39.828 17145 17145 F DEBUG : uid: 10582
03-07 15:48:39.828 17145 17145 F DEBUG : tagged_addr_ctrl: 0000000000000001 (PR_TAGGED_ADDR_ENABLE)
03-07 15:48:39.828 17145 17145 F DEBUG : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr --------
03-07 15:48:39.828 17145 17145 F DEBUG : x0 0000000000000000 x1 00000000000042df x2 0000000000000006 x3 0000007c6b4838f0
03-07 15:48:39.828 17145 17145 F DEBUG : x4 0000000000000000 x5 0000000000000000 x6 0000000000000000 x7 0000007f36a58004
03-07 15:48:39.828 17145 17145 F DEBUG : x8 00000000000000f0 x9 0000007f18a6d350 x10 0000000000000001 x11 0000007f18abe170
03-07 15:48:39.828 17145 17145 F DEBUG : x12 0000000000000004 x13 51c34e921a529271 x14 0000000000000001 x15 0000000000000001
03-07 15:48:39.828 17145 17145 F DEBUG : x16 0000007f18b24fd0 x17 0000007f18b10560 x18 0000007c694dc000 x19 00000000000042a2
03-07 15:48:39.828 17145 17145 F DEBUG : x20 00000000000042df x21 00000000ffffffff x22 0000001a580093f8 x23 0000007c6b483e18
03-07 15:48:39.828 17145 17145 F DEBUG : x24 0000000000000010 x25 0000000000000014 x26 0000001a5bc27ab8 x27 0000001a5bc27b40
03-07 15:48:39.828 17145 17145 F DEBUG : x28 0000001a56005b38 x29 0000007c6b483970
03-07 15:48:39.828 17145 17145 F DEBUG : lr 0000007f18aa78b8 sp 0000007c6b4838d0 pc 0000007f18aa78e4 pst 0000000000001000
03-07 15:48:39.828 17145 17145 F DEBUG : 2 total frames
03-07 15:48:39.828 17145 17145 F DEBUG : backtrace:
03-07 15:48:39.828 17145 17145 F DEBUG : #00 pc 000000000005d8e4 /apex/com.android.runtime/lib64/bionic/libc.so (abort+164) (BuildId: 1d36f8ae6e0af6158793abea7d4f4f2b)
03-07 15:48:39.828 17145 17145 F DEBUG : #01 pc 0000000000034828 /data/app/~~rqTsxkU5NjJYcDmnf-0haw==/Mono.Android.NET_Tests-fIi1xEY1rqvY2p4EDO81Ww==/split_config.arm64_v8a.apk (offset 0xa6c000)
The problem is that the `Android.App.Application` static constructor
was running *too early*, which caused an
`Android.Runtime.XAPeerMembers` instance to be constructed
*before the runtime had finished initialization*.
Which leads us to a digression: when is static constructor run?
The answer to this depends upon the [`beforefieldinit` flag][0]:
* When `beforefieldinit` is set, the runtime has lots of
flexibility over when the static constructor is executed.
The static constructor could be executed far earlier than one may
normally expect.
* When `beforefieldinit` *is not* set, the static constructor
is executed "immediately" before the type is used.
In C#, if a type does not contain a static constructor, then the
type has `beforefieldinit` set:
// has beforefieldinit
partial class Application {
static readonly JniPeerMembers _members = new XAPeerMembers ("android/app/Application", typeof (Application));
}
If a type has a static constructor, then it does not have
`beforefieldinit` set:
// has beforefieldinit
partial class Application {
static readonly JniPeerMembers _members = new XAPeerMembers ("android/app/Application", typeof (Application));
static Application()
{
}
}
We hit this same scenario in b6c22f39, and fixed that by using
`[MethodImpl(MethodImplOptions.NoInlining)]` on
`SetSynchronizationContext()`, on the hope that doing so would delay
execution of the `Application` static constructor until the invocation
of `SetSynchronizationContext()`. This worked for b6c22f39, but not
within NativeAOT.
To fix this:
* Add an empty static constructor to `Android.App.Application` to
remove the `beforefieldinit` flag from the type.
* Call `JNIEnvInit.SetSynchronizationContext()` at startup for
NativeAOT.
With this change in place, no crashes occur at startup and the test
passes.
[0]: https://csharpindepth.com/Articles/BeforeFieldInit |
@jonathanpeppers: would you be able to provide the TODO in my draft commit message? |
It is a native crash, not very descriptive...
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: b6c22f3909
Running
Mono.Android-Tests
under NativeAOT has the test failure:Initially, if we called
JNIEnvInit.SetSynchronizationContext()
at startup, this can result in the static ctor forAndroid.App.Application
running too soon beforeJNIEnvInit.InitializeJniRuntime()
has completed.To fix this:
Add an empty
static ctor
toAndroid.App.Application
to remove thebeforefieldinit
flag from the type.Call
JNIEnvInit.SetSynchronizationContext()
at startup for NativeAOT.With this change in place, no crashes occur at startup and the test passes.