Skip to content

Commit

Permalink
[monodroid] Prevent overlapped decompression of embedded assemblies (d…
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@grendello
grendello authored Jan 26, 2023
1 parent f04671a commit 2ff72d7
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 83 deletions.
14 changes: 7 additions & 7 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.apkdesc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
"Size": 1024
},
"assemblies/Java.Interop.dll": {
"Size": 58916
"Size": 58913
},
"assemblies/Mono.Android.dll": {
"Size": 87170
"Size": 87106
},
"assemblies/Mono.Android.Runtime.dll": {
"Size": 5931
"Size": 5860
},
"assemblies/rc.bin": {
"Size": 1182
Expand All @@ -35,7 +35,7 @@
"Size": 2265
},
"assemblies/UnnamedProject.dll": {
"Size": 3257
"Size": 3258
},
"classes.dex": {
"Size": 19020
Expand All @@ -44,7 +44,7 @@
"Size": 93552
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 434704
"Size": 379320
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 3089272
Expand All @@ -59,7 +59,7 @@
"Size": 152960
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 16760
"Size": 16720
},
"META-INF/BNDLTOOL.RSA": {
"Size": 1213
Expand Down Expand Up @@ -95,5 +95,5 @@
"Size": 1904
}
},
"PackageSize": 2648394
"PackageSize": 2632010
}
18 changes: 9 additions & 9 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleLegacy.apkdesc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,22 @@
"Size": 2604
},
"assemblies/Java.Interop.dll": {
"Size": 68923
"Size": 68913
},
"assemblies/Mono.Android.dll": {
"Size": 265140
"Size": 265170
},
"assemblies/mscorlib.dll": {
"Size": 769036
"Size": 769019
},
"assemblies/System.Core.dll": {
"Size": 28216
"Size": 28199
},
"assemblies/System.dll": {
"Size": 9192
"Size": 9180
},
"assemblies/UnnamedProject.dll": {
"Size": 2897
"Size": 2881
},
"classes.dex": {
"Size": 370828
Expand All @@ -32,7 +32,7 @@
"Size": 750976
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 333128
"Size": 277744
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 4039176
Expand All @@ -41,7 +41,7 @@
"Size": 66184
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 21264
"Size": 21256
},
"META-INF/ANDROIDD.RSA": {
"Size": 1213
Expand Down Expand Up @@ -74,5 +74,5 @@
"Size": 1724
}
},
"PackageSize": 4003540
"PackageSize": 3987156
}
12 changes: 6 additions & 6 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.apkdesc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
"Size": 7313
},
"assemblies/Java.Interop.dll": {
"Size": 66790
"Size": 66786
},
"assemblies/Mono.Android.dll": {
"Size": 444768
"Size": 444701
},
"assemblies/Mono.Android.Runtime.dll": {
"Size": 5931
"Size": 5860
},
"assemblies/mscorlib.dll": {
"Size": 3863
Expand Down Expand Up @@ -206,7 +206,7 @@
"Size": 93552
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 434704
"Size": 379320
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 3089272
Expand All @@ -221,7 +221,7 @@
"Size": 152960
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 333760
"Size": 333720
},
"META-INF/android.support.design_material.version": {
"Size": 12
Expand Down Expand Up @@ -1976,5 +1976,5 @@
"Size": 341228
}
},
"PackageSize": 7824132
"PackageSize": 7807748
}
76 changes: 38 additions & 38 deletions ...ild.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsLegacy.apkdesc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,112 +5,112 @@
"Size": 3140
},
"assemblies/FormsViewGroup.dll": {
"Size": 7230
"Size": 7215
},
"assemblies/Java.Interop.dll": {
"Size": 69966
"Size": 69955
},
"assemblies/Mono.Android.dll": {
"Size": 572670
"Size": 572710
},
"assemblies/Mono.Security.dll": {
"Size": 68449
"Size": 68432
},
"assemblies/mscorlib.dll": {
"Size": 915425
"Size": 915407
},
"assemblies/System.Core.dll": {
"Size": 164059
"Size": 164046
},
"assemblies/System.dll": {
"Size": 388883
"Size": 388865
},
"assemblies/System.Drawing.Common.dll": {
"Size": 12370
"Size": 12365
},
"assemblies/System.Net.Http.dll": {
"Size": 110718
"Size": 110693
},
"assemblies/System.Numerics.dll": {
"Size": 15706
"Size": 15683
},
"assemblies/System.Runtime.Serialization.dll": {
"Size": 186683
"Size": 186660
},
"assemblies/System.ServiceModel.Internals.dll": {
"Size": 26604
"Size": 26593
},
"assemblies/System.Xml.dll": {
"Size": 395668
"Size": 395657
},
"assemblies/UnnamedProject.dll": {
"Size": 116997
"Size": 116986
},
"assemblies/Xamarin.AndroidX.Activity.dll": {
"Size": 7711
"Size": 7697
},
"assemblies/Xamarin.AndroidX.AppCompat.AppCompatResources.dll": {
"Size": 6664
"Size": 6648
},
"assemblies/Xamarin.AndroidX.AppCompat.dll": {
"Size": 125346
"Size": 125328
},
"assemblies/Xamarin.AndroidX.CardView.dll": {
"Size": 7380
"Size": 7366
},
"assemblies/Xamarin.AndroidX.CoordinatorLayout.dll": {
"Size": 18289
"Size": 18272
},
"assemblies/Xamarin.AndroidX.Core.dll": {
"Size": 131944
"Size": 131930
},
"assemblies/Xamarin.AndroidX.DrawerLayout.dll": {
"Size": 15443
"Size": 15426
},
"assemblies/Xamarin.AndroidX.Fragment.dll": {
"Size": 43150
"Size": 43135
},
"assemblies/Xamarin.AndroidX.Legacy.Support.Core.UI.dll": {
"Size": 6727
"Size": 6715
},
"assemblies/Xamarin.AndroidX.Lifecycle.Common.dll": {
"Size": 7078
"Size": 7062
},
"assemblies/Xamarin.AndroidX.Lifecycle.LiveData.Core.dll": {
"Size": 7208
"Size": 7194
},
"assemblies/Xamarin.AndroidX.Lifecycle.ViewModel.dll": {
"Size": 4886
"Size": 4873
},
"assemblies/Xamarin.AndroidX.Loader.dll": {
"Size": 13596
"Size": 13585
},
"assemblies/Xamarin.AndroidX.RecyclerView.dll": {
"Size": 102349
"Size": 102326
},
"assemblies/Xamarin.AndroidX.SavedState.dll": {
"Size": 6294
"Size": 6268
},
"assemblies/Xamarin.AndroidX.SwipeRefreshLayout.dll": {
"Size": 11284
"Size": 11272
},
"assemblies/Xamarin.AndroidX.ViewPager.dll": {
"Size": 19438
"Size": 19424
},
"assemblies/Xamarin.Forms.Core.dll": {
"Size": 524743
"Size": 524736
},
"assemblies/Xamarin.Forms.Platform.Android.dll": {
"Size": 384885
"Size": 384872
},
"assemblies/Xamarin.Forms.Platform.dll": {
"Size": 56878
},
"assemblies/Xamarin.Forms.Xaml.dll": {
"Size": 55807
"Size": 55801
},
"assemblies/Xamarin.Google.Android.Material.dll": {
"Size": 43514
"Size": 43497
},
"classes.dex": {
"Size": 3533252
Expand All @@ -122,7 +122,7 @@
"Size": 750976
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 333128
"Size": 277744
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 4039176
Expand All @@ -131,7 +131,7 @@
"Size": 66184
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 107032
"Size": 107024
},
"META-INF/android.support.design_material.version": {
"Size": 12
Expand Down Expand Up @@ -1883,5 +1883,5 @@
"Size": 341040
}
},
"PackageSize": 9537694
"PackageSize": 9521310
}
Loading

0 comments on commit 2ff72d7

Please sign in to comment.