[monodroid] Remove __XA_DSO_IN_APK in favor of runtime checks (dotnet…

…#3337)

Fixes: dotnet#3298
Context: dotnet#3298 (comment)

We have discovered through our current preview of Android App Bundles
in 16.2: the state of `//application/@android:extractNativeLibs` with
app bundles can change on a per-device basis.

[A comment from `bundletool`'s source code][0]:

	// If persistent app is not installable on external storage, only the split APKs targeting
	// devices above Android M should be uncompressed. If the persistent app is installable on
	// external storage only split APKs targeting device above Android P should be uncompressed (as
	// uncompressed native libraries crashes with ASEC external storage and support for ASEC
	// external storage is removed in Android P). Instant apps always support uncompressed native
	// libraries (even on Android L), because they are not always executed by the Android platform.
	boolean shouldCompress =

That means we cannot decide the value of `__XA_DSO_IN_APK` at build
time, or know what the appropriate value should be!

Instead, we can check the value of [`ApplicationInfo.flags`][1] at
runtime for [`FLAG_EXTRACT_NATIVE_LIBS`][2].

We will have to check this value on the Java side, and pass it
through `Runtime.init()`.

This means we can kill the `__XA_DSO_IN_APK` environment variable
completely.

Other changes:

  * I added to add some explicit padding for 64-bit architectures
    after removing the `uses_embedded_dsos` value

  * I added a new `Runtime.initInternal()` method so we can adjust
    parameters without breaking the designer.  `Runtime.init()` can
    remain with its signature unchanged.

  * I adjusted all tests around embedded DSOs appropriately.

[0]: https://github.com/google/bundletool/blob/5ac94cb61e949f135c50f6ce52bbb5f00e8e959f/src/main/java/com/android/tools/build/bundletool/splitters/NativeLibrariesCompressionSplitter.java#L75-L86
[1]: https://developer.android.com/reference/android/content/pm/ApplicationInfo.html#flags
[2]: https://developer.android.com/reference/android/content/pm/ApplicationInfo.html#FLAG_EXTRACT_NATIVE_LIBS

Documentation/guides/app-bundles.md

@@ -245,11 +245,14 @@ older API levels:
     // always support uncompressed native libraries (even on Android L), because they are not always
     // executed by the Android platform.
 
-This means that developer's `extractNativeLibs` setting in their
-`AndroidManifest.xml` is basically ignored. See [bundletool's source
-code][nativelibs] for details.
+A developer's `extractNativeLibs` setting in `AndroidManifest.xml` is
+basically ignored. To make matters worse, on some devices the value
+will be set and others not. This means we cannot rely on a known value
+at build time.
 
-[nativelibs]: https://github.com/google/bundletool/blob/fe1129820cb263b3fef18ab7e95d80c228c065a1/src/main/java/com/android/tools/build/bundletool/splitters/NativeLibrariesCompressionSplitter.java#L74-L78
+See [bundletool's source code][nativelibs] for details.
+
+[nativelibs]: https://github.com/google/bundletool/blob/5ac94cb61e949f135c50f6ce52bbb5f00e8e959f/src/main/java/com/android/tools/build/bundletool/splitters/NativeLibrariesCompressionSplitter.java#L75-L86
 
 ## Signing
 

Documentation/guides/extract-native-libraries.md

@@ -42,8 +42,7 @@ Xamarin.Android build system will do the following:
     extension, causing native libraries to be stored uncompressed
     within the `.apk`.
 
- 2. The `__XA_DSO_IN_APK` environment variable will be set within the
-    created `.apk` file with the value of `1`, indicating to
-    the app that native libraries should be loaded from the `.apk`
-    itself instead of from the filesystem.
-
+At runtime we need to check `ApplicationInfo.flags` for
+`FLAG_EXTRACT_NATIVE_LIBS`. At build time, the state of
+`extractNativeLibs` is not for certain, since Android App Bundles
+change this value on a per-device basis.

src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs

@@ -137,7 +137,6 @@ public override bool Execute ()
 
 		void AddEnvironment ()
 		{
-			bool usesEmbeddedDSOs = false;
 			bool usesMonoAOT = false;
 			bool usesAssemblyPreload = EnablePreloadAssembliesDefault;
 			uint monoAOTMode = 0;
@@ -180,10 +179,6 @@ void AddEnvironment ()
 						haveHttpMessageHandler = true;
 					if (lineToWrite.StartsWith ("XA_TLS_PROVIDER=", StringComparison.Ordinal))
 						haveTlsProvider = true;
-					if (lineToWrite.StartsWith ("__XA_DSO_IN_APK", StringComparison.Ordinal)) {
-						usesEmbeddedDSOs = true;
-						continue;
-					}
 					if (lineToWrite.StartsWith ("mono.enable_assembly_preload=", StringComparison.Ordinal)) {
 						int idx = lineToWrite.IndexOf ('=');
 						uint val;
@@ -258,7 +253,6 @@ void AddEnvironment ()
 
 					var asmgen = new ApplicationConfigNativeAssemblyGenerator (asmTargetProvider, environmentVariables, systemProperties) {
 						IsBundledApp = IsBundledApplication,
-						UsesEmbeddedDSOs = usesEmbeddedDSOs,
 						UsesMonoAOT = usesMonoAOT,
 						UsesMonoLLVM = EnableLLVM,
 						UsesAssemblyPreload = usesAssemblyPreload,

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs

@@ -228,15 +228,6 @@ void AssertEmbeddedDSOs (string apk)
 				foreach (var entry in zip) {
 					if (entry.FullName.EndsWith (".so")) {
 						Assert.AreEqual (entry.Size, entry.CompressedSize, $"`{entry.FullName}` should be uncompressed!");
-					} else if (entry.FullName == "environment") {
-						using (var stream = new MemoryStream ()) {
-							entry.Extract (stream);
-							stream.Position = 0;
-							using (var reader = new StreamReader (stream)) {
-								string environment = reader.ReadToEnd ();
-								StringAssert.Contains ("__XA_DSO_IN_APK=1", environment, "`__XA_DSO_IN_APK=1` should be set via @(AndroidEnvironment)");
-							}
-						}
 					}
 				}
 			}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs

@@ -19,14 +19,13 @@ public sealed class ApplicationConfig
 		{
 			public bool   uses_mono_llvm;
 			public bool   uses_mono_aot;
-			public bool   uses_embedded_dsos;
 			public bool   uses_assembly_preload;
 			public bool   is_a_bundled_app;
 			public uint   environment_variable_count;
 			public uint   system_property_count;
 			public string android_package_name;
 		};
-		const uint ApplicationConfigFieldCount = 8;
+		const uint ApplicationConfigFieldCount = 7;
 
 		static readonly object ndkInitLock = new object ();
 		static readonly char[] readElfFieldSeparator = new [] { ' ', '\t' };
@@ -119,32 +118,27 @@ static ApplicationConfig ReadApplicationConfig (string envFile)
 						ret.uses_mono_aot = ConvertFieldToBool ("uses_mono_aot", envFile, i, field [1]);
 						break;
 
-					case 2: // uses_embedded_dsos: bool / .byte
-						AssertFieldType (envFile, ".byte", field [0], i);
-						ret.uses_embedded_dsos = ConvertFieldToBool ("uses_embedded_dsos", envFile, i, field [1]);
-						break;
-
-					case 3: // uses_assembly_preload: bool / .byte
+					case 2: // uses_assembly_preload: bool / .byte
 						AssertFieldType (envFile, ".byte", field [0], i);
 						ret.uses_assembly_preload = ConvertFieldToBool ("uses_assembly_preload", envFile, i, field [1]);
 						break;
 
-					case 4: // is_a_bundled_app: bool / .byte
+					case 3: // is_a_bundled_app: bool / .byte
 						AssertFieldType (envFile, ".byte", field [0], i);
 						ret.is_a_bundled_app = ConvertFieldToBool ("is_a_bundled_app", envFile, i, field [1]);
 						break;
 
-					case 5: // environment_variable_count: uint32_t / .word | .long
+					case 4: // environment_variable_count: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile}:{i}': {field [0]}");
 						ret.environment_variable_count = ConvertFieldToUInt32 ("environment_variable_count", envFile, i, field [1]);
 						break;
 
-					case 6: // system_property_count: uint32_t / .word | .long
+					case 5: // system_property_count: uint32_t / .word | .long
 						Assert.IsTrue (expectedUInt32Types.Contains (field [0]), $"Unexpected uint32_t field type in '{envFile}:{i}': {field [0]}");
 						ret.system_property_count = ConvertFieldToUInt32 ("system_property_count", envFile, i, field [1]);
 						break;
 
-					case 7: // android_package_name: string / [pointer type]
+					case 6: // android_package_name: string / [pointer type]
 						Assert.IsTrue (expectedPointerTypes.Contains (field [0]), $"Unexpected pointer field type in '{envFile}:{i}': {field [0]}");
 						pointers.Add (field [1].Trim ());
 						break;
@@ -288,7 +282,6 @@ static void AssertApplicationConfigIsIdentical (ApplicationConfig firstAppConfig
 		{
 			Assert.AreEqual (firstAppConfig.uses_mono_llvm, secondAppConfig.uses_mono_llvm, $"Field 'uses_mono_llvm' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'");
 			Assert.AreEqual (firstAppConfig.uses_mono_aot, secondAppConfig.uses_mono_aot, $"Field 'uses_mono_aot' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'");
-			Assert.AreEqual (firstAppConfig.uses_embedded_dsos, secondAppConfig.uses_embedded_dsos, $"Field 'uses_embedded_dsos' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'");
 			Assert.AreEqual (firstAppConfig.is_a_bundled_app, secondAppConfig.is_a_bundled_app, $"Field 'is_a_bundled_app' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'");
 			Assert.AreEqual (firstAppConfig.environment_variable_count, secondAppConfig.environment_variable_count, $"Field 'environment_variable_count' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'");
 			Assert.AreEqual (firstAppConfig.system_property_count, secondAppConfig.system_property_count, $"Field 'system_property_count' has different value in environment file '{secondEnvFile}' than in environment file '{firstEnvFile}'");

src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs

@@ -11,7 +11,6 @@ class ApplicationConfigNativeAssemblyGenerator : NativeAssemblyGenerator
 		uint stringCounter = 0;
 
 		public bool IsBundledApp { get; set; }
-		public bool UsesEmbeddedDSOs { get; set; }
 		public bool UsesMonoAOT { get; set; }
 		public bool UsesMonoLLVM { get; set; }
 		public bool UsesAssemblyPreload { get; set; }
@@ -48,9 +47,6 @@ protected override void WriteSymbols (StreamWriter output)
 				WriteCommentLine (output, "uses_mono_aot");
 				size += WriteData (output, UsesMonoAOT);
 
-				WriteCommentLine (output, "uses_embedded_dsos");
-				size += WriteData (output, UsesEmbeddedDSOs);
-
 				WriteCommentLine (output, "uses_assembly_preload");
 				size += WriteData (output, UsesAssemblyPreload);
 
@@ -63,6 +59,11 @@ protected override void WriteSymbols (StreamWriter output)
 				WriteCommentLine (output, "system_property_count");
 				size += WriteData (output, systemProperties == null ? 0 : systemProperties.Count * 2);
 
+				// After uses_embedded_dsos was removed, we need padding on 64-bit
+				if (TargetProvider.Is64Bit) {
+					size += WriteDataPadding (output, 4);
+				}
+
 				WriteCommentLine (output, "android_package_name");
 				size += WritePointer (output, stringLabel);
 

src/Xamarin.Android.Build.Tasks/Utilities/NativeAssemblyGenerator.cs

@@ -164,6 +164,11 @@ uint WriteDataPadding (StreamWriter output, uint nextFieldSize, uint sizeSoFar)
 				return 0;
 
 			uint nbytes = structureAlignBytes - alignment;
+			return WriteDataPadding (output, nbytes);
+		}
+
+		protected uint WriteDataPadding (StreamWriter output, uint nbytes)
+		{
 			output.WriteLine ($"{Indent}.zero{Indent}{nbytes}");
 			return nbytes;
 		}

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

@@ -2352,30 +2352,10 @@ because xbuild doesn't support framework reference assemblies.
     <Output TaskParameter="UsesLibraries"       ItemName="AndroidExternalJavaLibrary" />
   </ReadAndroidManifest>
   <PropertyGroup>
-    <!--NOTE: bundletool requires this, regardless of AndroidManifest.xml-->
-    <_EmbeddedDSOsEnabled Condition=" '$(AndroidPackageFormat)' == 'aab' ">True</_EmbeddedDSOsEnabled>
+    <AndroidStoreUncompressedFileExtensions Condition=" '$(_EmbeddedDSOsEnabled)' == 'True' ">.so;$(AndroidStoreUncompressedFileExtensions)</AndroidStoreUncompressedFileExtensions>
   </PropertyGroup>
 </Target>
 
-<Target Name="_SetupEmbeddedDSOs"
-    DependsOnTargets="_ReadAndroidManifest"
-    Condition=" '$(_EmbeddedDSOsEnabled)' == 'True' "
-    Inputs="$(IntermediateOutputPath)android\AndroidManifest.xml"
-    Outputs="$(IntermediateOutputPath)dsoenvironment.txt">
-  <WriteLinesToFile
-      File="$(IntermediateOutputPath)dsoenvironment.txt"
-      Lines="__XA_DSO_IN_APK=1"
-      Overwrite="True"
-  />
-  <PropertyGroup>
-    <AndroidStoreUncompressedFileExtensions>.so;$(AndroidStoreUncompressedFileExtensions)</AndroidStoreUncompressedFileExtensions>
-  </PropertyGroup>
-  <ItemGroup>
-    <AndroidEnvironment Include="$(IntermediateOutputPath)dsoenvironment.txt" />
-    <FileWrites         Include="$(IntermediateOutputPath)dsoenvironment.txt" />
-  </ItemGroup>
-</Target>
-
 <Target Name="_SetupAssemblyPreload"
     Condition=" '$(AndroidEnablePreloadAssemblies)' != 'True' ">
   <WriteLinesToFile
@@ -2398,7 +2378,7 @@ because xbuild doesn't support framework reference assemblies.
   </PrepareAbiItems>
 </Target>
 
-<Target Name="_GenerateEnvironmentFiles" DependsOnTargets="_ReadAndroidManifest;_SetupEmbeddedDSOs;_SetupAssemblyPreload" />
+<Target Name="_GenerateEnvironmentFiles" DependsOnTargets="_ReadAndroidManifest;_SetupAssemblyPreload" />
 
 <PropertyGroup>
   <_GeneratePackageManagerJavaDependsOn>

src/java-runtime/java/mono/android/MonoPackageManager.java

@@ -15,6 +15,9 @@
 
 public class MonoPackageManager {
 
+	// Exists only since API23 onwards, we must define it here
+	static final int FLAG_EXTRACT_NATIVE_LIBS = 0x10000000;
+
 	static Object lock = new Object ();
 	static boolean initialized;
 
@@ -45,6 +48,7 @@ public static void LoadApplication (Context context, ApplicationInfo runtimePack
 				String externalLegacyDir = new java.io.File (
 							external0,
 							"../legacy/Android/data/" + context.getPackageName () + "/files/.__override__").getAbsolutePath ();
+				boolean embeddedDSOsEnabled = (runtimePackage.flags & FLAG_EXTRACT_NATIVE_LIBS) == 0;
 
 				// Preload DSOs libmonodroid.so depends on so that the dynamic
 				// linker can resolve them when loading monodroid. This is not
@@ -54,7 +58,7 @@ public static void LoadApplication (Context context, ApplicationInfo runtimePack
 				System.loadLibrary("xamarin-app");
 				System.loadLibrary("monodroid");
 
-				Runtime.init (
+				Runtime.initInternal (
 						language,
 						apks,
 						getNativeLibraryPath (runtimePackage),
@@ -69,9 +73,8 @@ public static void LoadApplication (Context context, ApplicationInfo runtimePack
 							externalLegacyDir
 						},
 						MonoPackageManager_Resources.Assemblies,
-						null, // unused
 						android.os.Build.VERSION.SDK_INT,
-						null // unused
+						embeddedDSOsEnabled
 					);
 
 				mono.android.app.ApplicationRegistration.registerApplications ();

src/java-runtime/java/mono/android/Runtime.java

@@ -12,6 +12,7 @@ private Runtime ()
 	}
 
 	public static native void init (String lang, String[] runtimeApks, String runtimeDataDir, String[] appDirs, ClassLoader loader, String[] externalStorageDirs, String[] assemblies, String packageName, int apiLevel, String[] environmentVariables);
+	public static native void initInternal (String lang, String[] runtimeApks, String runtimeDataDir, String[] appDirs, ClassLoader loader, String[] externalStorageDirs, String[] assemblies, int apiLevel, boolean embeddedDSOsEnabled);
 	public static native void register (String managedType, java.lang.Class nativeClass, String methods);
 	public static native void notifyTimeZoneChanged ();
 	public static native int createNewContext (String[] runtimeApks, String[] assemblies, ClassLoader loader);

src/monodroid/jni/android-system.h

@@ -37,10 +37,10 @@ namespace xamarin { namespace android { namespace internal
 		static std::mutex readdir_mutex;
 		static char *libmonoandroid_directory_path;
 #endif
-
-// _WIN32 is defined with _WIN64 so _WIN64 must be checked first.
-#if __SIZEOF_POINTER__ == 8 || defined (_WIN64)
-#define __BITNESS__ "64bit"
+
+// _WIN32 is defined with _WIN64 so _WIN64 must be checked first.
+#if __SIZEOF_POINTER__ == 8 || defined (_WIN64)
+#define __BITNESS__ "64bit"
 #elif __SIZEOF_POINTER__ == 4 || defined (_WIN32)
 #define __BITNESS__ "32bit"
 #else
@@ -151,7 +151,12 @@ namespace xamarin { namespace android { namespace internal
 
 		bool is_embedded_dso_mode_enabled () const
 		{
-			return application_config.uses_embedded_dsos;
+			return embedded_dso_mode_enabled;
+		}
+
+		void set_embedded_dso_mode_enabled (bool yesno)
+		{
+			embedded_dso_mode_enabled = yesno;
 		}
 
 		MonoAotMode get_mono_aot_mode () const
@@ -197,6 +202,7 @@ namespace xamarin { namespace android { namespace internal
 	private:
 		int max_gref_count = 0;
 		MonoAotMode aotMode = MonoAotMode::MONO_AOT_MODE_NONE;
+		bool embedded_dso_mode_enabled = false;
 	};
 }}}
 #endif // !__ANDROID_SYSTEM_H

src/monodroid/jni/application_dso_stub.cc

@@ -15,7 +15,6 @@ uint8_t mj_typemap[] = { 0 };
 ApplicationConfig application_config = {
 	.uses_mono_llvm = false,
 	.uses_mono_aot = false,
-	.uses_embedded_dsos = false,
 	.uses_assembly_preload = false,
 	.is_a_bundled_app = false,
 	.environment_variable_count = 0,

src/monodroid/jni/monodroid-glue.cc

@@ -1864,11 +1864,24 @@ create_and_initialize_domain (JNIEnv* env, jclass runtimeClass, jstring_array_wr
 	return domain;
 }
 
+/* !DO NOT REMOVE! Used by the Android Designer */
 JNIEXPORT void JNICALL
 Java_mono_android_Runtime_init (JNIEnv *env, jclass klass, jstring lang, jobjectArray runtimeApksJava,
                                 jstring runtimeNativeLibDir, jobjectArray appDirs, jobject loader,
                                 jobjectArray externalStorageDirs, jobjectArray assembliesJava, jstring packageName,
                                 jint apiLevel, jobjectArray environmentVariables)
+{
+	Java_mono_android_Runtime_initInternal (
+		env, klass, lang, runtimeApksJava, runtimeNativeLibDir, 
+		appDirs, loader, externalStorageDirs, assembliesJava, apiLevel,
+		/* embeddedDSOsEnabled */ JNI_FALSE);
+}
+
+JNIEXPORT void JNICALL
+Java_mono_android_Runtime_initInternal (JNIEnv *env, jclass klass, jstring lang, jobjectArray runtimeApksJava,
+                                jstring runtimeNativeLibDir, jobjectArray appDirs, jobject loader,
+                                jobjectArray externalStorageDirs, jobjectArray assembliesJava,
+                                jint apiLevel, jboolean embeddedDSOsEnabled)
 {
 	init_logging_categories ();
 
@@ -1878,6 +1891,7 @@ Java_mono_android_Runtime_init (JNIEnv *env, jclass klass, jstring lang, jobject
 	}
 
 	android_api_level = apiLevel;
+	androidSystem.set_embedded_dso_mode_enabled ((bool) embeddedDSOsEnabled);
 
 	TimeZone_class = utils.get_class_from_runtime_field (env, klass, "java_util_TimeZone", true);
 

src/monodroid/jni/xamarin-app.h

@@ -18,7 +18,6 @@ struct ApplicationConfig
 {
 	bool uses_mono_llvm;
 	bool uses_mono_aot;
-	bool uses_embedded_dsos;
 	bool uses_assembly_preload;
 	bool is_a_bundled_app;
 	uint32_t environment_variable_count;

tests/EmbeddedDSOs/EmbeddedDSO-UnitTests/BuildTests.cs

@@ -112,7 +112,6 @@ public void EnvironmentFileContents ()
 			List<string> envFiles = EnvironmentHelper.GatherEnvironmentFiles (intermediateOutputDir, Xamarin.Android.Tools.XABuildConfig.SupportedABIs, true);
 			EnvironmentHelper.ApplicationConfig app_config = EnvironmentHelper.ReadApplicationConfig (envFiles);
 			Assert.That (app_config, Is.Not.Null, "application_config must be present in the environment files");
-			Assert.That (app_config.uses_embedded_dsos, Is.True, "'uses_embedded_dsos' must be true in application_config");
 		}
 
 		[Test]