[native] When preloading shared JNI libraries, update alias entries too (#10444)

Context: cba39dc
Context: #10376
Context: #10324

cba39dc implemented preloading of JNI-using native libraries but it missed to update
alias entries for each preloaded library.

During application build we generate native code that contains cache for each native
library packaged with the managed application code. Each library follows the same
naming pattern: `lib<NAME>.so`. However, the managed code can refer to those libraries
(when e.g. declaring a p/invoke with the `[DllImporrt]` attribute) using different forms
of names. The request may have a form of `lib<NAME>` or `<NAME>` etc. 

When the runtime tries to resolve the p/invoke symbol, it first needs to load the shared
library. This is done (in our case) by using a callback into our runtime which then tries
to find the library and load it. Should the attempt fail, the runtime will mutate the
library name and ask as again until all the possible names are tried or the library is
loaded successfully. This roundtrip is pretty expensive, so in our native library loader
code we implemented (in c227042) a scheme where at build time we mutate library names
ourselves and a separate entry for each name mutation in the shared library cache. This
way, when the runtime request comes, we perform a single search and are able to find
the library no matter what name the managed code requested.

Each of the cache entries contains, among other things irrelevant to this PR, a field
which stores the native library's handle, after it is loaded. cba39dc loaded the
library and set that field in just a single cache entry, the one corresponding to the
canonical library name (`lib<NAME>.so`) but it failed to set the field in all the aliases.
This resulted in an attempt to load the library again, with the managed code requesting it
by a different name, finding the corresponding cache entry and seeing that its handle is
unset. However, since the request was sent from a different thread, we attempted to load
the library on the main thread (described in detail in cba39dc commit message), which
attempt always failed leading to an endless loop and application crash/hang while debugging.

Fix the issue by setting native shared library handle in all the cache entries corresponding
to various mutations of the library name. This makes sure that further requests to load the
library will see the handle set in cache and use it, instead of attempting to load the it
again.

Author: grendello

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Forms/MainActivity.cs

@@ -23,5 +23,6 @@ protected override void OnCreate (Bundle savedInstanceState)
 			//${AFTER_FORMS_INIT}
 			LoadApplication (new App ());
 		}
+		//${AFTER_ONCREATE}
 	}
 }

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

@@ -161,7 +161,9 @@ sealed class DsoCacheState
 		{
 			public List<StructureInstance<DSOCacheEntry>> DsoCache = [];
 			public List<DSOCacheEntry> JniPreloadDSOs = [];
+			public List<string> JniPreloadNames = [];
 			public List<StructureInstance<DSOCacheEntry>> AotDsoCache = [];
+			public uint NameMutationsCount = 1;
 		}
 
 		// Keep in sync with FORMAT_TAG in src/monodroid/jni/xamarin-app.hh
@@ -274,10 +276,14 @@ protected override void Construct (LlvmIrModule module)
 			};
 			module.Add (dso_cache);
 
+			module.AddGlobalVariable ("dso_jni_preloads_idx_stride", dsoState.NameMutationsCount);
+
 			// This variable MUST be written after `dso_cache` since it relies on sorting performed by HashAndSortDSOCache
 			var dso_jni_preloads_idx = new LlvmIrGlobalVariable (new List<uint> (), "dso_jni_preloads_idx", LlvmIrVariableOptions.GlobalConstant) {
 				Comment = " Indices into dso_cache[] of DSO libraries to preload because of JNI use",
 				ArrayItemCount = (uint)dsoState.JniPreloadDSOs.Count,
+				GetArrayItemCommentCallback = GetPreloadIndicesLibraryName,
+				GetArrayItemCommentCallbackCallerState = dsoState,
 				BeforeWriteCallback = PopulatePreloadIndices,
 				BeforeWriteCallbackCallerState = dsoState,
 			};
@@ -346,6 +352,21 @@ void AddAssemblyStores (LlvmIrModule module)
 			module.Add (assembly_store);
 		}
 
+		string? GetPreloadIndicesLibraryName (LlvmIrVariable v, LlvmIrModuleTarget target, ulong index, object? value, object? callerState)
+		{
+			// Instead of throwing for such a triviality like a comment, we will return error messages as comments instead
+			var dsoState = callerState as DsoCacheState;
+			if (dsoState == null) {
+				return " Internal error: DSO state not present.";
+			}
+
+			if (index >= (ulong)dsoState.JniPreloadNames.Count) {
+				return $" Invalid index {index}";
+			}
+
+			return $" {dsoState.JniPreloadNames[(int)index]}";
+		}
+
 		void PopulatePreloadIndices (LlvmIrVariable variable, LlvmIrModuleTarget target, object? state)
 		{
 			var indices = variable.Value as List<uint>;
@@ -358,6 +379,9 @@ void PopulatePreloadIndices (LlvmIrVariable variable, LlvmIrModuleTarget target,
 				throw new InvalidOperationException ($"Internal error: DSO state not present.");
 			}
 
+			var dsoNames = new List<string> ();
+
+			// Indices array MUST NOT be sorted, since it groups alias entries together with the main entry
 			foreach (DSOCacheEntry preload in dsoState.JniPreloadDSOs) {
 				int dsoIdx = dsoState.DsoCache.FindIndex (entry => {
 					if (entry.Instance == null) {
@@ -372,8 +396,9 @@ void PopulatePreloadIndices (LlvmIrVariable variable, LlvmIrModuleTarget target,
 				}
 
 				indices.Add ((uint)dsoIdx);
+				dsoNames.Add (preload.HashedName ?? String.Empty);
 			}
-			indices.Sort ();
+			dsoState.JniPreloadNames = dsoNames;
 		}
 
 		void HashAndSortDSOCache (LlvmIrVariable variable, LlvmIrModuleTarget target, object? state)
@@ -427,15 +452,20 @@ DsoCacheState InitDSOCache ()
 			var jniPreloads = new List<DSOCacheEntry> ();
 			var aotDsoCache = new List<StructureInstance<DSOCacheEntry>> ();
 			var nameMutations = new List<string> ();
+			int nameMutationsCount = -1;
 
 			for (int i = 0; i < dsos.Count; i++) {
 				string name = dsos[i].name;
 
 				bool isJniLibrary = ELFHelper.IsJniLibrary (Log, dsos[i].item.ItemSpec);
 				bool ignore = dsos[i].ignore;
+				bool ignore_for_preload = !ApplicationConfigNativeAssemblyGeneratorCLR.DsoCacheJniPreloadIgnore.Contains (name);
 
 				nameMutations.Clear();
 				AddNameMutations (name);
+				if (nameMutationsCount == -1) {
+					nameMutationsCount = nameMutations.Count;
+				}
 
 				// All mutations point to the actual library name, but have hash of the mutated one
 				foreach (string entryName in nameMutations) {
@@ -447,23 +477,27 @@ DsoCacheState InitDSOCache ()
 						name = name,
 					};
 
-					if (entry.is_jni_library && entry.HashedName == name && !ApplicationConfigNativeAssemblyGeneratorCLR.DsoCacheJniPreloadIgnore.Contains (name)) {
-						jniPreloads.Add (entry);
-					}
-
 					var item = new StructureInstance<DSOCacheEntry> (dsoCacheEntryStructureInfo, entry);
 					if (name.StartsWith ("libaot-", StringComparison.OrdinalIgnoreCase)) {
 						aotDsoCache.Add (item);
-					} else {
-						dsoCache.Add (item);
+						continue;
 					}
+
+					// We must add all aliases to the preloads indices array so that all of them have their handle
+					// set when the library is preloaded.
+					if (entry.is_jni_library && ignore_for_preload) {
+						jniPreloads.Add (entry);
+					}
+
+					dsoCache.Add (item);
 				}
 			}
 
 			return new DsoCacheState {
 				DsoCache = dsoCache,
 				AotDsoCache = aotDsoCache,
 				JniPreloadDSOs = jniPreloads,
+				NameMutationsCount = (uint)(nameMutationsCount <= 0 ? 1 : nameMutationsCount),
 			};
 
 			void AddNameMutations (string name)

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

@@ -243,8 +243,10 @@ sealed class DsoCacheState
 	{
 		public List<StructureInstance<DSOCacheEntry>> DsoCache = [];
 		public List<DSOCacheEntry> JniPreloadDSOs = [];
+		public List<string> JniPreloadNames = [];
 		public List<StructureInstance<DSOCacheEntry>> AotDsoCache = [];
 		public LlvmIrStringBlob NamesBlob = null!;
+		public uint NameMutationsCount = 1;
 	}
 
 	// Keep in sync with FORMAT_TAG in src/monodroid/jni/xamarin-app.hh
@@ -394,10 +396,14 @@ protected override void Construct (LlvmIrModule module)
 		};
 		module.Add (dso_cache);
 
+		module.AddGlobalVariable ("dso_jni_preloads_idx_stride", dsoState.NameMutationsCount);
+
 		// This variable MUST be written after `dso_cache` since it relies on sorting performed by HashAndSortDSOCache
 		var dso_jni_preloads_idx = new LlvmIrGlobalVariable (new List<uint> (), "dso_jni_preloads_idx", LlvmIrVariableOptions.GlobalConstant) {
 			Comment = " Indices into dso_cache[] of DSO libraries to preload because of JNI use",
 			ArrayItemCount = (uint)dsoState.JniPreloadDSOs.Count,
+			GetArrayItemCommentCallback = GetPreloadIndicesLibraryName,
+			GetArrayItemCommentCallbackCallerState = dsoState,
 			BeforeWriteCallback = PopulatePreloadIndices,
 			BeforeWriteCallbackCallerState = dsoState,
 		};
@@ -566,6 +572,21 @@ void AddAssemblyStores (LlvmIrModule module)
 		module.Add (assembly_store);
 	}
 
+	string? GetPreloadIndicesLibraryName (LlvmIrVariable v, LlvmIrModuleTarget target, ulong index, object? value, object? callerState)
+	{
+		// Instead of throwing for such a triviality like a comment, we will return error messages as comments instead
+		var dsoState = callerState as DsoCacheState;
+		if (dsoState == null) {
+			return " Internal error: DSO state not present.";
+		}
+
+		if (index >= (ulong)dsoState.JniPreloadNames.Count) {
+			return $" Invalid index {index}";
+		}
+
+		return $" {dsoState.JniPreloadNames[(int)index]}";
+	}
+
 	void PopulatePreloadIndices (LlvmIrVariable variable, LlvmIrModuleTarget target, object? state)
 	{
 		var indices = variable.Value as List<uint>;
@@ -578,6 +599,9 @@ void PopulatePreloadIndices (LlvmIrVariable variable, LlvmIrModuleTarget target,
 			throw new InvalidOperationException ($"Internal error: DSO state not present.");
 		}
 
+		var dsoNames = new List<string> ();
+
+		// Indices array MUST NOT be sorted, since it groups alias entries together with the main entry
 		foreach (DSOCacheEntry preload in dsoState.JniPreloadDSOs) {
 			int dsoIdx = dsoState.DsoCache.FindIndex (entry => {
 				if (entry.Instance == null) {
@@ -592,8 +616,9 @@ void PopulatePreloadIndices (LlvmIrVariable variable, LlvmIrModuleTarget target,
 			}
 
 			indices.Add ((uint)dsoIdx);
+			dsoNames.Add (preload.HashedName ?? String.Empty);
 		}
-		indices.Sort ();
+		dsoState.JniPreloadNames = dsoNames;
 	}
 
 	void HashAndSortDSOCache (LlvmIrVariable variable, LlvmIrModuleTarget target, object? state)
@@ -648,16 +673,22 @@ DsoCacheState InitDSOCache ()
 		var aotDsoCache = new List<StructureInstance<DSOCacheEntry>> ();
 		var nameMutations = new List<string> ();
 		var dsoNamesBlob = new LlvmIrStringBlob ();
+		int nameMutationsCount = -1;
 
 		for (int i = 0; i < dsos.Count; i++) {
 			string name = dsos[i].name;
 			(int nameOffset, _) = dsoNamesBlob.Add (name);
 
 			bool isJniLibrary = ELFHelper.IsJniLibrary (Log, dsos[i].item.ItemSpec);
 			bool ignore = dsos[i].ignore;
+			bool ignore_for_preload = !DsoCacheJniPreloadIgnore.Contains (name);
 
 			nameMutations.Clear();
 			AddNameMutations (name);
+			if (nameMutationsCount == -1) {
+				nameMutationsCount = nameMutations.Count;
+			}
+
 			// All mutations point to the actual library name, but have hash of the mutated one
 			foreach (string entryName in nameMutations) {
 				var entry = new DSOCacheEntry {
@@ -670,16 +701,19 @@ DsoCacheState InitDSOCache ()
 					name_index = (uint)nameOffset,
 				};
 
-				if (entry.is_jni_library && entry.HashedName == name && !DsoCacheJniPreloadIgnore.Contains (name)) {
-					jniPreloads.Add (entry);
-				}
-
 				var item = new StructureInstance<DSOCacheEntry> (dsoCacheEntryStructureInfo, entry);
 				if (name.StartsWith ("libaot-", StringComparison.OrdinalIgnoreCase)) {
 					aotDsoCache.Add (item);
-				} else {
-					dsoCache.Add (item);
+					continue;
 				}
+
+				// We must add all aliases to the preloads indices array so that all of them have their handle
+				// set when the library is preloaded.
+				if (entry.is_jni_library && ignore_for_preload) {
+					jniPreloads.Add (entry);
+				}
+
+				dsoCache.Add (item);
 			}
 		}
 
@@ -688,6 +722,7 @@ DsoCacheState InitDSOCache ()
 			JniPreloadDSOs = jniPreloads,
 			AotDsoCache = aotDsoCache,
 			NamesBlob = dsoNamesBlob,
+			NameMutationsCount = (uint)(nameMutationsCount <= 0 ? 1 : nameMutationsCount),
 		};
 
 		void AddNameMutations (string name)

src/native/clr/host/host.cc

@@ -334,6 +334,60 @@ auto Host::create_delegate (
 	return delegate;
 }
 
+[[gnu::flatten, gnu::always_inline]]
+void Host::preload_jni_libraries () noexcept
+{
+	// NOTE: when fixing a bug here, fix also the MonoVM code in src/native/mono/monodroid-glue.cc@preload_jni_libraries
+	if (application_config.number_of_shared_libraries == 0) [[unlikely]] {
+		return;
+	}
+
+	log_debug (LOG_ASSEMBLY, "DSO jni preloads index stride == {}", dso_jni_preloads_idx_stride);
+
+	if ((dso_jni_preloads_idx_count % dso_jni_preloads_idx_stride) != 0) [[unlikely]] {
+		Helpers::abort_application (
+			LOG_ASSEMBLY,
+			std::format (
+				"DSO preload index is invalid, size ({}) is not a multiple of {}"sv,
+				dso_jni_preloads_idx_count,
+				dso_jni_preloads_idx_stride
+			)
+		);
+	}
+
+	for (size_t i = 0; i < dso_jni_preloads_idx_count; i += dso_jni_preloads_idx_stride) {
+		const size_t entry_index = dso_jni_preloads_idx[i];
+		DSOCacheEntry &entry = dso_cache[entry_index];
+		const std::string_view dso_name = MonodroidDl::get_dso_name (&entry);
+
+		log_debug (
+			LOG_ASSEMBLY,
+			"Preloading JNI shared library: {} (entry's index: {}; real name hash: {:x}; name hash: {:x})",
+			dso_name,
+			entry_index,
+			entry.real_name_hash,
+			entry.hash
+		);
+
+		void *handle = MonodroidDl::monodroid_dlopen (&entry, dso_name, RTLD_NOW);
+
+		// Set handle in all the alias entries
+		for (size_t j = 1; j < dso_jni_preloads_idx_stride; j++) {
+			const size_t entry_alias_index = dso_jni_preloads_idx[i + j];
+			DSOCacheEntry &entry_alias = dso_cache[entry_alias_index];
+			const std::string_view entry_alias_name = MonodroidDl::get_dso_name (&entry);
+
+			log_debug (
+				LOG_ASSEMBLY,
+				"Putting JNI library handle in alias entry at index {}: {}",
+				entry_alias_index,
+				entry_alias_name
+			);
+			entry_alias.handle = handle;
+		}
+	}
+}
+
 void Host::Java_mono_android_Runtime_initInternal (
 	JNIEnv *env, jclass runtimeClass, jstring lang, jobjectArray runtimeApksJava,
 
@@ -433,12 +487,7 @@ void Host::Java_mono_android_Runtime_initInternal (
 		gettid ()
 	);
 
-	for (size_t i = 0; i < dso_jni_preloads_idx_count; i++) {
-		DSOCacheEntry &entry = dso_cache[dso_jni_preloads_idx[i]];
-		const std::string_view dso_name = MonodroidDl::get_dso_name (&entry);
-		log_debug (LOG_ASSEMBLY, "Preloading JNI shared library: {}", dso_name);
-		MonodroidDl::monodroid_dlopen (&entry, dso_name, RTLD_NOW);
-	}
+	preload_jni_libraries ();
 
 	struct JnienvInitializeArgs init = {};
 	init.javaVm                                         = jvm;

src/native/clr/include/host/host.hh

@@ -49,6 +49,8 @@ namespace xamarin::android {
 			std::string_view const& assembly_name, std::string_view const& type_name,
 			std::string_view const& method_name) noexcept -> void*;
 
+		static void preload_jni_libraries () noexcept;
+
 	private:
 		static inline void *clr_host = nullptr;
 		static inline unsigned int domain_id = 0;

src/native/clr/include/xamarin-app.hh

@@ -348,6 +348,7 @@ extern "C" {
 	[[gnu::visibility("default")]] extern AssemblyStoreRuntimeData assembly_store;
 
 	[[gnu::visibility("default")]] extern DSOCacheEntry dso_cache[];
+	[[gnu::visibility("default")]] extern const uint dso_jni_preloads_idx_stride;
 	[[gnu::visibility("default")]] extern const uint dso_jni_preloads_idx_count;
 	[[gnu::visibility("default")]] extern const uint dso_jni_preloads_idx[];
 	[[gnu::visibility("default")]] extern DSOCacheEntry aot_dso_cache[];

src/native/clr/xamarin-app-stub/application_dso_stub.cc

@@ -127,6 +127,7 @@ DSOCacheEntry dso_cache[] = {
 	},
 };
 
+const uint dso_jni_preloads_idx_stride = 1;
 const uint dso_jni_preloads_idx_count = 1;
 const uint dso_jni_preloads_idx[1] = {
 	0

src/native/mono/monodroid/monodroid-glue-internal.hh

@@ -162,6 +162,7 @@ namespace xamarin::android::internal
 		static void monodroid_unhandled_exception (MonoObject *java_exception);
 		static MonoClass* get_android_runtime_class () noexcept;
 
+		static void preload_jni_libraries () noexcept;
 		static MonoDomain* create_domain (JNIEnv *env, jstring_array_wrapper &runtimeApks, bool is_root_domain, bool have_split_apks) noexcept;
 		static MonoDomain* create_and_initialize_domain (JNIEnv* env, jclass runtimeClass, jstring_array_wrapper &runtimeApks,
 		                                          jstring_array_wrapper &assemblies, jobjectArray assembliesBytes, jstring_array_wrapper &assembliesPaths,