Skip to content
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

Bump manifest-merger from 30.0.4 to 30.1.0 in /src/manifestmerger #4

Conversation

dependabot[bot]
Copy link

@dependabot dependabot bot commented on behalf of github Jan 31, 2022

Bumps manifest-merger from 30.0.4 to 30.1.0.

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Bumps manifest-merger from 30.0.4 to 30.1.0.

---
updated-dependencies:
- dependency-name: com.android.tools.build:manifest-merger
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Jan 31, 2022
jonathanpeppers added a commit that referenced this pull request Feb 3, 2022
…6672)

Context: dotnet/maui#4262
Context: dotnet#6675

If you run the `maui-blazor` template in a Release build:

	dotnet build -t:Run -c Release

it crashes at runtime:

	D monodroid-assembly: typemap: type with token 33555274 (0x200034a) in module {C7B4CC8F-7A03-4A3F-A34A-DC66EDC548B9} (Mono.Android) corresponds to Java type 'android/runtime/JavaProxyThrowable'
	…
	F DEBUG   : backtrace:
	F DEBUG   : #00 pc 000000000065d8fc  /apex/com.android.art/lib64/libart.so (void art::StackVisitor::WalkStack<(art::StackVisitor::CountTransitions)0>(bool)+156) (BuildId: 7fbaf2a1a3317bd634b00eb90e32291e)
	F DEBUG   : #1 pc 000000000069b25d  /apex/com.android.art/lib64/libart.so (art::Thread::GetCurrentMethod(unsigned int*, bool, bool) const+157) (BuildId: 7fbaf2a1a3317bd634b00eb90e32291e)
	F DEBUG   : #2 pc 0000000000430fed  /apex/com.android.art/lib64/libart.so (art::JNI<false>::FindClass(_JNIEnv*, char const*)+765) (BuildId: 7fbaf2a1a3317bd634b00eb90e32291e)
	F DEBUG   : #3 pc 0000000000047e5a  /data/app/~~0Qm6D1S0sO3f1lwfakN0PA==/com.companyname.mauiapp2-08UokVCH5k_PlbZEH_hhkA==/split_config.x86_64.apk!libmono-android.release.so (offset 0x11e000) (java_interop_jnienv_find_class+26) (BuildId: 3d04f8b946590175e97b89aee2e3b19ceed4b524)
	F DEBUG   : #4 pc 00000000000128ac  <anonymous:41640000>

The crash can be avoided by disabling the linker:

	dotnet build -t:Run -c Release -p:AndroidLinkMode=None
	# -or-
	dotnet build -t:Run -c Release -p:PublishTrimmed=false

However, let us return to the crash: *why* is it crashing?
This isn't a "good debugging experience"; we have no useful context.

Lots of investigation later -- all hail printf debugging -- and we
found that the cause of the crash was an unhandled exception:

 1. `Mono.Android.dll` has it's Java Callable Wrappers generated
    from the *unlinked* assembly, into `mono.android.jar` and
    `mono.android.dex` files.  The Java Callable Wrapper for
    `Android.Runtime.InputStreamAdapter` thus includes *all*
    `Read()` method overloads.

 2. When the app is built in Release configuration, linking is
    enabled, and *some* of the `InputStreamAdapter.Read()` methods
    are removed by the linker, along with the
    `Java.IO.InputStream.Read()` methods that were overridden.

 3. At runtime, we perform [Java Type Registration][0] for the
    `Android.Runtime.InputStreamAdapter` type, which eventually calls
    `AndroidTypeManager.RegisterNativeMembers()`, which eventually
    attempts to *effectively* do:

        Delegate.CreateDelegate (
	        typeof(Func<Delegate>),
	        typeof(InputStreamAdapter),
	        "GetReadHandler");

 4. Because of (2), `Java.IO.InputStream.GetReadHandler()`
    *does not exist*, and thus `Delegate.CreateDelegate()` throws an
    `ArgumentException`.

So far, so reasonable, but…

 5. `AndroidTypeManager.RegisterNativeMembers()` didn't catch any
    exceptions, nor did any other method between the original Java
    `Runtime.register()` invocation and
    `AndroidTypeManager.RegisterNativeMembers()`.  The result is that
    a C# exception was "in flight", and Mono then proceeded to
    *tear down the stack frame* as it unwound the callstack looking
    for `catch` handlers.

At this point, the process is toast: the runtime stack is FUBAR.

This is also why the `backtrace:` is "rooted" in
`JNIEnv::FindClass()`: `JNIEnv::FindClass()` invokes Java static
constructors before returning, which is how the static constructor in
the Java Callable Wrapper for `InputStreamAdapter` called
`Runtime.register()` in the first place.

All of this makes for a miserable debugging experience.

Fixing the "original" linker issue will be done in
dotnet#6675.

This hasn't been an issue in "Classic" Xamarin.Android, presumably
because the classic linker isn't as good as the net6 linker.

What we want to do *here* is improve this debugging experience, by
"wrapping" `AndroidTypeManager.RegisterNativeMembers()` in a
`try`/`catch` block, which can then *marshal the thrown exception*
back to Java.  This *prevents* Mono from unwinding the callstack past
a JNI boundary, and avoids the annoying-to-debug app crash.

After this change, we get a much friendlier unhandled exception crash:

	I MonoDroid: Android.Runtime.JavaProxyThrowable: Exception_WasThrown, Android.Runtime.JavaProxyThrowable
	I MonoDroid:
	I MonoDroid:   --- End of managed Android.Runtime.JavaProxyThrowable stack trace ---
	I MonoDroid: android.runtime.JavaProxyThrowable: System.ArgumentException: Arg_DlgtTargMeth
	I MonoDroid:    at System.Delegate.CreateDelegate(Type , Type , String , Boolean , Boolean )
	I MonoDroid:    at System.Delegate.CreateDelegate(Type , Type , String )
	I MonoDroid:    at Android.Runtime.AndroidTypeManager.RegisterNativeMembers(JniType , Type , String )
	I MonoDroid: --- End of stack trace from previous location ---
	I MonoDroid:    at Java.Interop.JniEnvironment.StaticMethods.CallStaticObjectMethod(JniObjectReference , JniMethodInfo , JniArgumentValue* )
	I MonoDroid:    at Android.Runtime.JNIEnv.CallStaticObjectMethod(IntPtr , IntPtr , JValue* )
	I MonoDroid:    at Android.Runtime.JNIEnv.CallStaticObjectMethod(IntPtr , IntPtr , JValue[] )
	I MonoDroid:    at Android.Runtime.JNIEnv.FindClass(String )
	I MonoDroid:    at Android.Runtime.JNIEnv.AllocObject(String )
	I MonoDroid:    at Android.Runtime.JNIEnv.StartCreateInstance(String , String , JValue* )
	I MonoDroid:    at Android.Runtime.JNIEnv.StartCreateInstance(String , String , JValue[] )
	I MonoDroid:    at Android.Runtime.InputStreamAdapter..ctor(Stream )
	I MonoDroid:    at Android.Runtime.InputStreamAdapter.ToLocalJniHandle(Stream )
	I MonoDroid:    at Android.Webkit.WebResourceResponse..ctor(String , String , Int32 , String , IDictionary`2 , Stream )
	I MonoDroid:    at Microsoft.AspNetCore.Components.WebView.Maui.WebKitWebViewClient.ShouldInterceptRequest(WebView view, IWebResourceRequest request)
	I MonoDroid:    at Android.Webkit.WebViewClient.n_ShouldInterceptRequest_Landroid_webkit_WebView_Landroid_webkit_WebResourceRequest_(IntPtr , IntPtr , IntPtr , IntPtr )
	I MonoDroid: 	at crc64d693e2d9159537db.WebKitWebViewClient.n_shouldInterceptRequest(Native Method)
	I MonoDroid: 	at crc64d693e2d9159537db.WebKitWebViewClient.shouldInterceptRequest(WebKitWebViewClient.java:39)
	I MonoDroid: 	at Rr.a(chromium-TrichromeWebViewGoogle.apk-stable-410410686:16)
	I MonoDroid: 	at org.chromium.android_webview.AwContentsBackgroundThreadClient.shouldInterceptRequestFromNative(chromium-TrichromeWebViewGoogle.apk-stable-410410686:2)
	I MonoDroid:
	I MonoDroid:   --- End of managed Android.Runtime.JavaProxyThrowable stack trace ---
	I MonoDroid: android.runtime.JavaProxyThrowable: System.ArgumentException: Arg_DlgtTargMeth
	I MonoDroid:    at System.Delegate.CreateDelegate(Type , Type , String , Boolean , Boolean )
	I MonoDroid:    at System.Delegate.CreateDelegate(Type , Type , String )
	I MonoDroid:    at Android.Runtime.AndroidTypeManager.RegisterNativeMembers(JniType , Type , String )
	I MonoDroid: --- End of stack trace from previous location ---
	I MonoDroid:    at Java.Interop.JniEnvironment.StaticMethods.CallStaticObjectMethod(JniObjectReference , JniMethodInfo , JniArgumentValue* )
	I MonoDroid:    at Android.Runtime.JNIEnv.CallStaticObjectMethod(IntPtr , IntPtr , JValue* )
	I MonoDroid:    at Android.Runtime.JNIEnv.CallStaticObjectMethod(IntPtr , IntPtr , JValue[] )
	I MonoDroid:    at Android.Runtime.JNIEnv.FindClass(String )
	I MonoDroid:    at Android.Runtime.JNIEnv.AllocObject(String )
	I MonoDroid:    at Android.Runtime.JNIEnv.StartCreateInstance(String , String , JValue* )
	I MonoDroid:    at Android.Runtime.JNIEnv.StartCreateInstance(String , String , JValue[] )
	I MonoDroid:    at Android.Runtime.InputStreamAdapter..ctor(Stream )
	I MonoDroid:    at Android.Runtime.InputStreamAdapter.ToLocalJniHandle(Stream )
	I MonoDroid:    at Android.Webkit.WebResourceResponse..ctor(String , String , Int32 , String , IDictionary`2 , Stream )
	I MonoDroid:    at Microsoft.AspNetCore.Components.WebView.Maui.WebKitWebViewClient.ShouldInterceptRequest(WebView view, IWebResourceRequest request)
	I MonoDroid:    at Android.Webkit.WebViewClient.n_ShouldInterceptRequest_Landroid_webkit_WebView_Landroid_webkit_WebResourceRequest_(IntPtr , IntPtr , IntPtr , IntPtr )
	I MonoDroid: 	at crc64d693e2d9159537db.WebKitWebViewClient.n_shouldInterceptRequest(Native Method)
	I MonoDroid: 	at crc64d693e2d9159537db.WebKitWebViewClient.shouldInterceptRequest(WebKitWebViewClient.java:39)
	I MonoDroid: 	at Rr.a(chromium-TrichromeWebViewGoogle.apk-stable-410410686:16)
	I MonoDroid: 	at org.chromium.android_webview.AwContentsBackgroundThreadClient.shouldInterceptRequestFromNative(chromium-TrichromeWebViewGoogle.apk-stable-410410686:2)

This is much easier to reason about, and will save us time in
the future.

[0]: https://github.com/xamarin/xamarin-android/wiki/Blueprint#java-type-registration
@dependabot @github
Copy link
Author

dependabot bot commented on behalf of github Feb 7, 2022

Superseded by #5.

@dependabot dependabot bot closed this Feb 7, 2022
@dependabot dependabot bot deleted the dependabot/gradle/src/manifestmerger/com.android.tools.build-manifest-merger-30.1.0 branch February 7, 2022 08:16
jonathanpeppers pushed a commit that referenced this pull request Jan 31, 2023
…otnet#7732)

Fixes: dotnet#7335

Context: d236af5

Commit d236af5 introduced embedded assembly compression, using LZ4,
which speeds up startup and reduces final package size.

Assemblies are compressed at build time and, at the same time, pre-
allocated buffers for the **decompressed** data are allocated in
`libxamarin-app.so`.  The buffers are then passed to the LZ4 APIs,
all threads using the same output buffer.  The assumption was that we
can do fine without locking as even if overlapped decompression
happens, the output data will be the same and so even if two threads
do the same thing at the same time, the data will be valid at all
times, so long as at least one thread completes the decompression.

This assumption proved to be **largely** true, but it appears that in
high concurrency cases it is possible that the data in the
decompression buffer differs.  This can result in app crashes:

	A/libc: Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 3092 (.NET ThreadPool), pid 2727 (myapp.name)
	A/DEBUG: pid: 2727, tid: 3092, name: .NET ThreadPool  >>> myapp.name <<<
	A/DEBUG:       #1 pc 0000000000029b1c  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmono-android.release.so (offset 0x103d000) (xamarin::android::internal::MonodroidRuntime::mono_log_handler(char const*, char const*, char const*, int, void*)+144) (BuildId: 29c5a3805a0bedee1eede9b6668d7c676aa63371)
	A/DEBUG:       #2 pc 00000000002680bc  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	A/DEBUG:       #3 pc 00000000002681e8  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	A/DEBUG:       #4 pc 000000000008555c  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (mono_metadata_string_heap+188) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	…

My guess is that LZ4 either uses the output buffer as a scratchpad
area when decompressing or that it initializes/modifies the buffer
before writing actual data in it.  With overlapped decompression, it
may lead to one thread overwriting valid data previously written by
another thread, so that when the latter returns the buffer it thought
to have had valid data may contain certain bytes temporarily
overwritten by the decompression session in the other, still running,
thread.  It may happen that MonoVM reads the corrupted data just when
it is still invalid (before the still running decompression session
actually writes the valid data), a classic race condition.

To fix this, the decompression block is now protected with a startup-
aware mutex.  Mutex will be held only after the initial startup phase
is completed, so there should not be much loss of startup performance.
jonathanpeppers pushed a commit that referenced this pull request Feb 27, 2023
…otnet#7817)

Fixes: dotnet#7335

Context: d236af5

Commit d236af5 introduced embedded assembly compression, using LZ4,
which speeds up startup and reduces final package size.

Assemblies are compressed at build time and, at the same time, pre-
allocated buffers for the **decompressed** data are allocated in
`libxamarin-app.so`.  The buffers are then passed to the LZ4 APIs,
all threads using the same output buffer.  The assumption was that we
can do fine without locking as even if overlapped decompression
happens, the output data will be the same and so even if two threads
do the same thing at the same time, the data will be valid at all
times, so long as at least one thread completes the decompression.

This assumption proved to be **largely** true, but it appears that in
high concurrency cases it is possible that the data in the
decompression buffer differs.  This can result in app crashes:

	A/libc: Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 3092 (.NET ThreadPool), pid 2727 (myapp.name)
	A/DEBUG: pid: 2727, tid: 3092, name: .NET ThreadPool  >>> myapp.name <<<
	A/DEBUG:       #1 pc 0000000000029b1c  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmono-android.release.so (offset 0x103d000) (xamarin::android::internal::MonodroidRuntime::mono_log_handler(char const*, char const*, char const*, int, void*)+144) (BuildId: 29c5a3805a0bedee1eede9b6668d7c676aa63371)
	A/DEBUG:       #2 pc 00000000002680bc  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	A/DEBUG:       #3 pc 00000000002681e8  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	A/DEBUG:       #4 pc 000000000008555c  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (mono_metadata_string_heap+188) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	…

My guess is that LZ4 either uses the output buffer as a scratchpad
area when decompressing or that it initializes/modifies the buffer
before writing actual data in it.  With overlapped decompression, it
may lead to one thread overwriting valid data previously written by
another thread, so that when the latter returns the buffer it thought
to have had valid data may contain certain bytes temporarily
overwritten by the decompression session in the other, still running,
thread.  It may happen that MonoVM reads the corrupted data just when
it is still invalid (before the still running decompression session
actually writes the valid data), a classic race condition.

To fix this, the decompression block is now protected with a startup-
aware mutex.  Mutex will be held only after the initial startup phase
is completed, so there should not be much loss of startup performance.
jonathanpeppers pushed a commit that referenced this pull request Jul 17, 2023
)

Context: 929e701
Context: ce2bc68
Context: dotnet#7473
Context: dotnet#8155

The managed linker can produce assemblies optimized for the target
`$(RuntimeIdentifier)` (RID), which means that they will differ
between different RIDs.  Our "favorite" example of this is
`IntPtr.Size`, which is inlined by the linker into `4` or `8` when
targeting 32-bit or 64-bit platforms.  (See also dotnet#7473 and 929e701.)

Another platform difference may come in the shape of CPU intrinsics
which will change the JIT-generated native code in ways that will
crash the application if the assembler instructions generated for the
intrinsics aren't supported by the underlying processor.

In addition, the per-RID assemblies will have different [MVID][0]s
and **may** have different type and method metadata token IDs, which
is important because typemaps *use* type and metadata token IDs; see
also ce2bc68.

All of this taken together invalidates our previous assumption that
all the managed assemblies are identical.  "Simply" using
`IntPtr.Size` in an assembly that contains `Java.Lang.Object`
subclasses will break things.

This in turn could cause "mysterious" behavior or crashes in Release
applications; see also Issue dotnet#8155.

Prevent the potential problems by processing each per-RID assembly
separately and output correct per-RID LLVM IR assembly using the
appropriate per-RID information.

Additionally, during testing I found that for our use of Cecil within
`<GenerateJavaStubs/>` doesn't consistently remove the fields,
delegates, and methods we remove in `MarshalMethodsAssemblyRewriter`
when marshal methods are enabled, or it generates subtly broken
assemblies which cause **some** applications to segfault at run time
like so:

	I monodroid-gc: 1 outstanding GREFs. Performing a full GC!
	F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x8 in tid 12379 (t6.helloandroid), pid 12379 (t6.helloandroid)
	F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
	F DEBUG   : Build fingerprint: 'google/raven_beta/raven:14/UPB3.230519.014/10284690:user/release-keys'
	F DEBUG   : Revision: 'MP1.0'
	F DEBUG   : ABI: 'arm64'
	F DEBUG   : Timestamp: 2023-07-04 22:09:58.762982002+0200
	F DEBUG   : Process uptime: 1s
	F DEBUG   : Cmdline: com.microsoft.net6.helloandroid
	F DEBUG   : pid: 12379, tid: 12379, name: t6.helloandroid  >>> com.microsoft.net6.helloandroid <<<
	F DEBUG   : uid: 10288
	F DEBUG   : tagged_addr_ctrl: 0000000000000001 (PR_TAGGED_ADDR_ENABLE)
	F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0000000000000008
	F DEBUG   : Cause: null pointer dereference
	F DEBUG   :     x0  0000000000000000  x1  0000007ba1401af0  x2  00000000000000fa  x3  0000000000000001
	F DEBUG   :     x4  0000007ba1401b38  x5  0000007b9f2a8360  x6  0000000000000000  x7  0000000000000000
	F DEBUG   :     x8  ffffffffffc00000  x9  0000007b9f800000  x10 0000000000000000  x11 0000007ba1400000
	F DEBUG   :     x12 0000000000000000  x13 0000007ba374ad58  x14 0000000000000000  x15 00000013ead77d66
	F DEBUG   :     x16 0000007ba372f210  x17 0000007ebdaa4a80  x18 0000007edf612000  x19 000000000000001f
	F DEBUG   :     x20 0000000000000000  x21 0000007b9f2a8320  x22 0000007b9fb02000  x23 0000000000000018
	F DEBUG   :     x24 0000007ba374ad08  x25 0000000000000004  x26 0000007b9f2a4618  x27 0000000000000000
	F DEBUG   :     x28 ffffffffffffffff  x29 0000007fc592a780
	F DEBUG   :     lr  0000007ba3701f44  sp  0000007fc592a730  pc  0000007ba3701e0c  pst 0000000080001000
	F DEBUG   : 8 total frames
	F DEBUG   : backtrace:
	F DEBUG   :       #00 pc 00000000002d4e0c  /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877)
	F DEBUG   :       #1 pc 00000000002c29e8  /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877)
	F DEBUG   :       #2 pc 00000000002c34bc  /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877)
	F DEBUG   :       #3 pc 00000000002c2254  /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877)
	F DEBUG   :       #4 pc 00000000002be0bc  /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877)
	F DEBUG   :       #5 pc 00000000002bf050  /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877)
	F DEBUG   :       #6 pc 00000000002a53a4  /data/app/~~Av24J15xbf20XdrY3X2_wA==/com.microsoft.net6.helloandroid-4DusuNWIAkz1Ssi7fWVF-g==/lib/arm64/libmonosgen-2.0.so (mono_gc_collect+44) (BuildId: 761134f2369377582cc3a8e25ecccb43a2e0a877)
	F DEBUG   :       #7 pc 000000000000513c  <anonymous:7ec716b000>

This is because we generate Java Callable Wrappers over a set of
original (linked or not) assemblies, then we scan them for classes
derived from `Java.Lang.Object` and use that set as input to the
marshal methods rewriter, which makes the changes (generates wrapper
methods, decorates wrapped methods with `[UnmanagedCallersOnly]`,
removes the old delegate methods as well as delegate backing fields)
to all the `Java.Lang.Object` subclasses, then writes the modified
assembly to a `new/<assembly.dll>` location (efa14e2), followed by
copying the newly written assemblies back to the original location.
At this point, we have the results returned by the subclass scanner
in memory and **new** versions of those types on disk, but they are
out of sync, since the types in memory refer to the **old** assemblies,
but AOT is ran on the **new** assemblies which have a different layout,
changed MVIDs and, potentially, different type and method token IDs
(because we added some methods, removed others etc) and thus it causes
the crashes at the run time.  The now invalid set of "old" types is
passed to the typemap generator.  This only worked by accident, because
we (incorrectly) used only the first linked assembly which happened
to be the same one passed to the JLO scanner and AOT - so everything
was fine at the execution time.

Address this by *disabling* LLVM Marshal Methods (8bc7a3e) for .NET 8,
setting `$(AndroidEnableMarshalMethods)`=False by default.
We'll attempt to fix these issues for .NET 9.

[0]: https://learn.microsoft.com/dotnet/api/system.reflection.module.moduleversionid?view=net-7.0
jonathanpeppers pushed a commit that referenced this pull request Aug 30, 2023
…otnet#7732)

Fixes: dotnet#7335

Context: d236af5

Commit d236af5 introduced embedded assembly compression, using LZ4,
which speeds up startup and reduces final package size.

Assemblies are compressed at build time and, at the same time, pre-
allocated buffers for the **decompressed** data are allocated in
`libxamarin-app.so`.  The buffers are then passed to the LZ4 APIs,
all threads using the same output buffer.  The assumption was that we
can do fine without locking as even if overlapped decompression
happens, the output data will be the same and so even if two threads
do the same thing at the same time, the data will be valid at all
times, so long as at least one thread completes the decompression.

This assumption proved to be **largely** true, but it appears that in
high concurrency cases it is possible that the data in the
decompression buffer differs.  This can result in app crashes:

	A/libc: Fatal signal 6 (SIGABRT), code -1 (SI_QUEUE) in tid 3092 (.NET ThreadPool), pid 2727 (myapp.name)
	A/DEBUG: pid: 2727, tid: 3092, name: .NET ThreadPool  >>> myapp.name <<<
	A/DEBUG:       #1 pc 0000000000029b1c  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmono-android.release.so (offset 0x103d000) (xamarin::android::internal::MonodroidRuntime::mono_log_handler(char const*, char const*, char const*, int, void*)+144) (BuildId: 29c5a3805a0bedee1eede9b6668d7c676aa63371)
	A/DEBUG:       #2 pc 00000000002680bc  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	A/DEBUG:       #3 pc 00000000002681e8  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	A/DEBUG:       #4 pc 000000000008555c  /data/app/myapp.name-B9t_3dF9i8mDxJEKodZw5w==/split_config.arm64_v8a.apk!libmonosgen-2.0.so (offset 0x109b000) (mono_metadata_string_heap+188) (BuildId: 4a5dd4396e8816b7f69881838bd549285213d53b)
	…

My guess is that LZ4 either uses the output buffer as a scratchpad
area when decompressing or that it initializes/modifies the buffer
before writing actual data in it.  With overlapped decompression, it
may lead to one thread overwriting valid data previously written by
another thread, so that when the latter returns the buffer it thought
to have had valid data may contain certain bytes temporarily
overwritten by the decompression session in the other, still running,
thread.  It may happen that MonoVM reads the corrupted data just when
it is still invalid (before the still running decompression session
actually writes the valid data), a classic race condition.

To fix this, the decompression block is now protected with a startup-
aware mutex.  Mutex will be held only after the initial startup phase
is completed, so there should not be much loss of startup performance.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants