Support DSOs embedded in the .apk #2154

grendello · 2018-09-06T20:58:53Z

Context: #1906

Support DSOs embedded in the APK

Android API 23 introduced a new way of dealing with the native shared libraries
shipped in the APK. Before that API level, the libraries would be always
extraced and placed in the application data directory, thus occupying more space
than necessary. API 23 added a new manifest <application> element attribute,
android:extractNativeLibs, which if set makes Android not extract the
libraries to the filesystem. API 23 added a way to load those libraries directly
from the APK. In order to support that there are a few requirements which this
commit implements:

DSO (.so) files must be stored uncompressed in the APK
<application android:extractNativeLibs="false"/> must be present
DSOs in the APK must be aligned on the memory page boundary (the -p flag
passed to zipalign takes care of that

This commit also implements libmonodroid suport for loading our DSOs directly
from the APK. This operation mode is enabled by the presence of the
__XA_DSO_IN_APK environment variable. This variable is inserted into the
application's environment by way of placing it in the environment file (a file
part of the XA project that is marked with the AndroidEnvironment build
action). In that mode, the DSOs are no longer looked up in the application
data directory but only in the override directories (if the APK is built in
Debug configuration) and in the APK itself.

Currently, in order to activate the above mode, one has to perform the following
actions manually:

Add the android:extractNativeLibs="false" attribute to the <application>
element in the Properties/AndroidManifest.xml file
Add the following property to the project file:

      <AndroidStoreUncompressedFileExtensions>.so</AndroidStoreUncompressedFileExtensions>

Add an android environment file to the project with a line which says

      __XA_DSO_IN_APK=1

After that the application should work in the embedded DSO mode without
problems.

A couple of tests are provided to test building and execution of embedded DSO
application on device, as well as to validate the built APK.

jonpryor · 2018-09-07T01:11:00Z

src/monodroid/jni/monodroid-glue.c

+get_full_dso_path (const char *base_dir, const char *dso_path, mono_bool *needs_free)
 {
-	void *libapp;
+	if (needs_free != NULL)


Alternate consideration: so that you don't need to repeat if (needs_free != null) *needs_free=..., you can have it point to a temporary instead:

mono_bool _def_needs_free; if (needs_free == NULL) needs_free = &_def_needs_free;

Then you can always access *needs_free without the check.

jonpryor · 2018-09-07T01:12:07Z

src/monodroid/jni/monodroid-glue.c

+		return NULL;

-	libapp = dlopen (libappso, RTLD_LAZY);
+	if (base_dir == NULL || dso_path [0] == MONODROID_PATH_SEPARATOR_CHAR)


(Since I haven't tried to check...)

Can get_full_dso_path() be called from the Windows/Designer codepaths? If so, this could be a Windows path. Perhaps we need an is_path_rooted(const char*) function?

Yeah, this is just a quick check "for now", to get the code working on device asap. I'll add the function to TODO

jonpryor · 2018-09-07T01:12:57Z

src/monodroid/jni/monodroid-glue.c


-	libapp = dlopen (libappso, RTLD_LAZY);
+	if (base_dir == NULL || dso_path [0] == MONODROID_PATH_SEPARATOR_CHAR)
+		return (char*)dso_path; // Absolute path, can't do much with it


Especially considering the eventual C++ migration, is this really a good idea, casting away const like this?

Should we instead make the dso_path non-const for sanity?

Yes and no. dso_path should be a const - we shouldn't modify it in any way. OTOH, the return value must be non-const since we may allocate it. In this case a conscious cast like this is OK - in the presence of the needs_free flag we make the intent explicit. The parameter const status is a promise we won't modify the parameter.

I think dso_path shouldn't be const here, as by the cast before returning it you remove the const and have no control of what happens with the returned value afterwards. And because the return value can be allocated and thus we need to call free on it later, you cannot return const char*. (or you can, but have to cast it when calling free, which I find less harmful as it is at the end of the life of it)

IIRC, once we switch to C++ you might return const pointer, as the delete operator can be used with the const pointer.

I think it's safer (now) to return char* and do the cast inside the function than rely on the caller to perform the cast - the caller has the boolean flag to tell them when to free the returned value. Granted, it's not ideal, but within the realm of C there's no good way to handle this situation. In C++ we can do what @radekdoulik suggests or use a wrapper class which will know what to do with the stored value.

src/monodroid/jni/monodroid-glue.c

jonpryor · 2018-09-07T01:27:18Z

src/monodroid/jni/monodroid-glue.c

-	if (!mono_mkbundle_initialize_mono_api)
-		log_error (LOG_BUNDLE, "Missing initialize_mono_api in the application");
+static void*
+load_dso (const char *path, int dl_flags, mono_bool skip_exists_check)


Again, as per previous digression, load_dso() implies -- to me -- that i'll load "from anywhere," while this only supports looking at filesystem locations. In keeping with the above, perhaps load_external_dso(), or load_file_dso()?

And my previous reply applies here as well :)

radekdoulik · 2018-09-07T06:43:11Z

src/monodroid/jni/monodroid-glue.c

-#else
-	log_fatal (LOG_DEFAULT, "cannot find libmonosgen-2.0.so in override_dir: %s, app_libdir: %s nor in previously printed locations.", override_dirs[0], app_libdir);
+	TRY_LIBMONOSGEN (SYSTEM_LIB_PATH);
+	log_fatal (LOG_DEFAULT, "Cannot find '%'. Looked in the following locations:", MONO_SGEN_SO);


Typo? '%' --> '%s'

👍, well spotted, thanks :)

grendello · 2018-09-07T06:48:34Z

Hmm, and the build failed because of

04:42:45  > git clean -fdx # timeout=10
04:43:11 FATAL: Command "git clean -fdx" returned status code 1:
04:43:11 Removing xamarin.android-oss_v9.1.99.161_Darwin-x86_64_HEAD_a9f0db2/
04:43:11 
04:43:11 stderr: warning: failed to remove bin/TestRelease/temp: Directory not empty
04:43:11 
04:43:11 Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to JNLP4-connect connection from 167.220.148.18/167.220.148.18:44113

grendello · 2018-09-13T07:05:29Z

The failure is because of a mono test crash that wasn't caught by NUnit:

09-12 20:24:06.297  4797  4815 I NUnit   : Read  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    : Unhandled Exception: (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    : System.IO.IOException: Unable to read data from the transport connection: Connection reset by peer. ---> System.Net.Sockets.SocketException: Connection reset by peer (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Net.Sockets.Socket.Receive (System.Byte[] buffer, System.Int32 offset, System.Int32 size, System.Net.Sockets.SocketFlags socketFlags) [0x00017] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referen
cesource/System/net/System/Net/Sockets/Socket.cs:1773  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Net.Sockets.NetworkStream.Read (System.Byte[] buffer, System.Int32 offset, System.Int32 size) [0x0009b] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/System/net/System/Net/Sockets/N
etworkStream.cs:513  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :    --- End of inner exception stack trace --- (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Net.Sockets.NetworkStream.Read (System.Byte[] buffer, System.Int32 offset, System.Int32 size) [0x000c3] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/System/net/System/Net/Sockets/N
etworkStream.cs:525  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Net.RequestStream.Read (System.Byte[] buffer, System.Int32 offset, System.Int32 count) [0x00033] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/System/System.Net/RequestStream.cs:141  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at MonoTests.System.Net.StreamExtensions.ReadAll (System.IO.Stream stream, System.Byte[] buffer, System.Int32 offset, System.Int32 count) [0x00004] in <506591a0ad94482da7f4d89d0939a426>:0  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at MonoTests.System.Net.HttpWebRequestTest+<>c__DisplayClass63_0.<PrematureStreamCloseAborts>b__1 (System.Net.HttpListenerContext c) [0x0000b] in <506591a0ad94482da7f4d89d0939a426>:0  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at MonoTests.System.Net.HttpWebRequestTest+ListenerScope+<>c__DisplayClass4_0.<RequestHandler>b__0 (System.Object o) [0x00000] in <506591a0ad94482da7f4d89d0939a426>:0  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Threading.QueueUserWorkItemCallback.WaitCallback_Context (System.Object state) [0x0000d] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadpool.cs:1308  
(TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Threading.ExecutionContext.RunInternal (System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, System.Object state, System.Boolean preserveSyncCtx) [0x00071] in /Users/builder/jenkins/workspace/xamarin-a
ndroid/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/executioncontext.cs:961  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Threading.ExecutionContext.Run (System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, System.Object state, System.Boolean preserveSyncCtx) [0x00000] in /Users/builder/jenkins/workspace/xamarin-android/x
amarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/executioncontext.cs:908  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem () [0x00021] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadp
ool.cs:1285  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Threading.ThreadPoolWorkQueue.Dispatch () [0x00074] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadpool.cs:858  (TaskId:131)
  09-12 20:24:06.297  4797  7962 E mono    :   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback () [0x00000] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadpool.cs:1213  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt : [ERROR] FATAL UNHANDLED EXCEPTION: System.IO.IOException: Unable to read data from the transport connection: Connection reset by peer. ---> System.Net.Sockets.SocketException: Connection reset by peer (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Net.Sockets.Socket.Receive (System.Byte[] buffer, System.Int32 offset, System.Int32 size, System.Net.Sockets.SocketFlags socketFlags) [0x00017] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referen
cesource/System/net/System/Net/Sockets/Socket.cs:1773  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Net.Sockets.NetworkStream.Read (System.Byte[] buffer, System.Int32 offset, System.Int32 size) [0x0009b] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/System/net/System/Net/Sockets/N
etworkStream.cs:513  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :    --- End of inner exception stack trace --- (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Net.Sockets.NetworkStream.Read (System.Byte[] buffer, System.Int32 offset, System.Int32 size) [0x000c3] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/System/net/System/Net/Sockets/N
etworkStream.cs:525  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Net.RequestStream.Read (System.Byte[] buffer, System.Int32 offset, System.Int32 count) [0x00033] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/System/System.Net/RequestStream.cs:141  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at MonoTests.System.Net.StreamExtensions.ReadAll (System.IO.Stream stream, System.Byte[] buffer, System.Int32 offset, System.Int32 count) [0x00004] in <506591a0ad94482da7f4d89d0939a426>:0  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at MonoTests.System.Net.HttpWebRequestTest+<>c__DisplayClass63_0.<PrematureStreamCloseAborts>b__1 (System.Net.HttpListenerContext c) [0x0000b] in <506591a0ad94482da7f4d89d0939a426>:0  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at MonoTests.System.Net.HttpWebRequestTest+ListenerScope+<>c__DisplayClass4_0.<RequestHandler>b__0 (System.Object o) [0x00000] in <506591a0ad94482da7f4d89d0939a426>:0  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Threading.QueueUserWorkItemCallback.WaitCallback_Context (System.Object state) [0x0000d] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadpool.cs:1308  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Threading.ExecutionContext.RunInternal (System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, System.Object state, System.Boolean preserveSyncCtx) [0x00071] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/executioncontext.cs:961  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Threading.ExecutionContext.Run (System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, System.Object state, System.Boolean preserveSyncCtx) [0x00000] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/executioncontext.cs:908  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem () [0x00021] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadpool.cs:1285  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Threading.ThreadPoolWorkQueue.Dispatch () [0x00074] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadpool.cs:858  (TaskId:131)
  09-12 20:24:06.298  4797  7962 E mono-rt :   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback () [0x00000] in /Users/builder/jenkins/workspace/xamarin-android/xamarin-android/external/mono/mcs/class/referencesource/mscorlib/system/threading/threadpool.cs:1213  (TaskId:131)

jonpryor · 2018-09-13T14:46:52Z

tests/EmbeddedDSOs/EmbeddedDSO-UnitTests/BuildTests.cs

+
+namespace EmbeddedDSOUnitTests
+{
+	sealed class LocalBuilder : Builder


I wonder if these unit tests should be integrated with/moved into src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests.

I kept them separate because the Android project used by those tests is also a standalone test in its own right (with instrumentation and such)

jonpryor · 2018-09-13T14:48:26Z

tests/EmbeddedDSOs/EmbeddedDSO/Tests.cs

+		[Test]
+		public void TestApplicationActuallyRan ()
+		{
+			// We need some test to run the instrumentation - this way we know that the runtime loads and


Yes, we need a test.

Here's a better one: assert that if we're running on API >= 23, that there are no lib*.so files in whatever /data/data/@PACKAGE_NAME@/lib is (using appropriate logic to find that path).

This would help avoid "false positives," where we think it's working, but it's not, because the .so files were extracted when we thought they wouldn't be.

jonpryor · 2018-09-13T14:49:41Z

tests/RunApkTests.targets

  <Import Project="..\tests\CodeGen-Binding\Xamarin.Android.JcwGen-Tests\Xamarin.Android.JcwGen-Tests.projitems" Condition=" '$(AotAssemblies)' != 'True' " />
  <Import Project="..\tests\locales\Xamarin.Android.Locale-Tests\Xamarin.Android.Locale-Tests.projitems"  Condition=" '$(AotAssemblies)' != 'True' " />
  <Import Project="..\tests\Xamarin.Forms-Performance-Integration\Droid\Xamarin.Forms.Performance.Integration.Droid.projitems"  Condition=" '$(Configuration)' == 'Release' And '$(AotAssemblies)' != 'true' " />
+  <Import Project="..\tests\EmbeddedDSOs\EmbeddedDSO\EmbeddedDSO.projitems" Condition=" '$(AotAssemblies)' != 'True' " />


This raises a good question:

Will this work with AOT'd assemblies? Meaning, does mono use us to find/load the appropriate libaot-mscorlib.dll.so/etc. files? I think it does...but it would probably be a good idea to run these tests when $(AotAssemblies)=True, to ensure that it works.

I think the Condition should be removed.

Yes, it does. See here for a debug output for this PR on Android O:

09-07 12:31:04.940 21101 21101 D Mono : Image addref mscorlib[0xf205fcc0] -> mscorlib.dll[0xf2a8cd00]: 2 09-07 12:31:04.941 21101 21101 D Mono : Prepared to set up assembly 'mscorlib' (mscorlib.dll) 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="mscorlib.dll.so", flags=0x3, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonosgen-2.0.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.941 21101 21101 D linker : ... dlopen failed: library "mscorlib.dll.so" not found 09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.941 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found 09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.941 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found 09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.941 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found 09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/mscorlib.dll.so" not found 09-07 12:31:04.942 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/mscorlib.dll.so" not found" 09-07 12:31:04.942 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.942 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.942 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.942 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.942 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found

Will this work with AOT'd assemblies?

Yes, it does

Yay.

However, your snippet doesn't answer my question: for loading AOT'd assemblies, I'd expect a message that contains a substring along the lines of base.apk!/lib/x86/libaot-mscorlib.dll.so. I do not see any message in your snippet which contains that substring. Instead, every mention of libaot-mscorlib.dll.so is for the .__override__ directory, not within the .apk. As such, the snippet does not appear to answer my question/concern.

Additionally, you didn't reply to my final suggestion:

I think the Condition should be removed.

Thoughts?

Here's the full log excerpt which contains loads from the apk. The lookup directories are set up in the runtime initialization function and they already contain base paths inside the APK archive in embedded mode. So the load_dso_from_any_directories will look in them:

09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.941 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found 09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.941 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found 09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.941 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found 09-07 12:31:04.941 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/mscorlib.dll.so" not found" 09-07 12:31:04.941 21101 21101 D linker : dlopen(name="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/mscorlib.dll.so" not found 09-07 12:31:04.942 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/mscorlib.dll.so" not found" 09-07 12:31:04.942 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.942 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.942 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.942 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.942 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.942 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.942 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.942 21101 21101 D linker : dlopen(name="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.943 21101 21101 D linker : ... dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libaot-mscorlib.dll.so" not found 09-07 12:31:04.943 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.943 21101 21101 D linker : dlopen(name="/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so", flags=0x3, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonosgen-2.0.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.943 21101 21101 D linker : ... dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found 09-07 12:31:04.943 21101 21101 D linker : dlerror set to "dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found" 09-07 12:31:04.943 21101 21101 D linker : dlopen(name="/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.943 21101 21101 D linker : ... dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found 09-07 12:31:04.943 21101 21101 D linker : dlerror set to "dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found" 09-07 12:31:04.943 21101 21101 D linker : dlopen(name="/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.943 21101 21101 D linker : ... dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found 09-07 12:31:04.943 21101 21101 D linker : dlerror set to "dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found" 09-07 12:31:04.943 21101 21101 D linker : dlopen(name="/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.943 21101 21101 D linker : ... dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found 09-07 12:31:04.943 21101 21101 D linker : dlerror set to "dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found" 09-07 12:31:04.943 21101 21101 D linker : dlopen(name="/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.943 21101 21101 D linker : ... dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found 09-07 12:31:04.944 21101 21101 D linker : dlerror set to "dlopen failed: library "/home/grendel/vc/xamarin/xamarin-android/external/mono/sdks/out/android-x86-debug/lib/mono/aot-cache/x86/mscorlib.dll.so" not found" 09-07 12:31:04.944 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.944 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.944 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.944 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.944 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.944 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.944 21101 21101 D linker : dlopen(name="/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.944 21101 21101 D linker : ... dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found 09-07 12:31:04.944 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/user/0/com.xamarin.NotificationChannels/files/.__override__/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.944 21101 21101 D linker : dlopen(name="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libaot-mscorlib.dll.so", flags=0x1, extinfo=(null), caller="/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libmonodroid.so", caller_ns=classloader-namespace@0xf7579110) ... 09-07 12:31:04.944 21101 21101 D linker : ... dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libaot-mscorlib.dll.so" not found 09-07 12:31:04.944 21101 21101 D linker : dlerror set to "dlopen failed: library "/data/app/com.xamarin.NotificationChannels-WCBF9YhPQcjyUVKBgGNbYA==/base.apk!/lib/x86/libaot-mscorlib.dll.so" not found" 09-07 12:31:04.962 21101 21101 D Mono : Assembly mscorlib[0xf205fcc0] added to domain RootDomain, ref_count=1

Regarding the condtion - I don't know why it was put in there for all the other projects, so I can't make a decision :)

@grendello wrote:

Regarding the condtion - I don't know why it was put in there for all the other projects, so I can't make a decision

The Condition is in there to not "unduly" prolong the build, because we believe that there isn't sufficient merit in running those tests under AOT scenarios (because AOT is time consuming).

Note that Mono.Android-Tests.projitems doesn't have the Condition, and thus is run for all configurations, because we believe there is merit in running that test under AOT.

I think that there is merit in running EmbeddedDSO.projitems when $(AotAssemblies)==True, because this test is very low-level and could easily break AOT if not done carefully.

As such, I think we should do:

<Import Project="..\tests\EmbeddedDSOs\EmbeddedDSO\EmbeddedDSO.projitems" />

jonpryor · 2018-09-13T15:02:18Z

src/monodroid/jni/monodroid-glue.c

 static int android_api_level = 0;

+// Values correspond to the CPU_KIND_* macros
+static char* android_abi_names[] = {


I think it might be handy to explicitly provide the array size:

static const char* android_abi_names[CPU_KIND_X86_64+1] = {

That way if we add a CPU_KIND_FOO value and an element to the array, we'll get a warning unless we update the array size:

/* some time later... */ #define CPU_KIND_X86_64 ((unsigned short)6) /* ... */ static const char *android_abi_names[CPU_KIND_X86_64+1] = { /* ... */ "foo", /* added */ }; /* results in a compiler warning: warning: excess elements in array initializer "foo", ^--- */

What this won't do is ensure that indexes match as we expect. clang supports this syntax; not sure about gcc/C99:

static const char * android_abi_names[CPU_KIND_X86_64+1] = { "*unknown*", [CPU_KIND_ARM] = "armeabi-v7a", [CPU_KIND_ARM64] = "arm64-v8a", [CPU_KIND_MIPS] = "mips", [CPU_KIND_X86] = "x86", [CPU_KIND_X86_64] = "x86_64", };

jonpryor · 2018-09-13T15:03:11Z

src/monodroid/jni/monodroid-glue.c

+	"x86",
+	"x86_64",
+};
+#define ANDROID_ABI_NAMES_SIZE (sizeof(android_abi_names) / sizeof (char*))


"conventional" is to use (sizeof(android_abi_names) / sizeof (android_abi_names[0])), so that if the element type changes, ANDROID_ABI_NAMES_SIZE doesn't need to change.

jonpryor · 2018-09-13T15:13:51Z

src/monodroid/jni/monodroid-glue.c

+	}
 #endif
-	TRY_LIBMONOSGEN (app_libdir)
+	if (!embedded_dso_mode) {


This condition concerns me, in that it means that if a Debug build was using "embedded DSO support," that Debug build can't use Improved Fast Deployment, because we only look in the override directories when embedded_dso_mode==0.

No, this is incorrect. We look in override dirs in embedded mode. See the following:

https://github.com/grendello/xamarin-android/blob/issue1906/src/monodroid/jni/monodroid-glue.c#L890
https://github.com/grendello/xamarin-android/blob/issue1906/src/monodroid/jni/monodroid-glue.c#L3278
https://github.com/grendello/xamarin-android/blob/issue1906/src/monodroid/jni/monodroid-glue.c#L3293
https://github.com/grendello/xamarin-android/blob/issue1906/src/monodroid/jni/monodroid-glue.c#L3494
https://github.com/grendello/xamarin-android/blob/issue1906/src/monodroid/jni/monodroid-glue.c#L3996

I tested this PR in Debug mode :)

I tested this PR in Debug mode :)

Which is wonderful! The problem is that my "static analysis" (trying to read this PR) doesn't easily see why that works, not easily, not obviously. Here, within get_libmonosgen_path(), when embedded_dso_mode==TRUE, how do we find .__override__/.../libmonosgen-2.0.so? Do we find libmonosgen-2.0.so within this function at all, or is it found "elsewhere"? If it is "elsewhere," where is that "elsewhere"?

Turns out, that "elsewhere" is in Java_mono_android_Runtime_init():

if (embedded_dso_mode) { libmonosgen_handle = load_dso_from_any_directories (MONO_SGEN_SO, sgen_dlopen_flags); }

I wonder if things would be clear if instead of a get_libmonosgen_path() we had a load_libmonosgen(), so that the "find libmonosgen" and "load libmonosgen" logic weren't so far apart. :-/

Yes, that would be better - this code could use a lot of improvements all over the map. However, I don't want to muddle the actual topic of this particular PR with logic and code cleanup. I'd like to make such changes/improvements after we merge in the C++ PR. I purposefully stayed as close to original code when implementing this feature as possible (even though I often felt like rewriting large portions of what I saw - but that would mean going down the rabbit hole and making this PR something that cannot be properly reviewed)

src/monodroid/jni/monodroid-glue.c

jonpryor · 2018-09-13T19:20:04Z

src/monodroid/jni/monodroid-glue.c

-		free (full_name);
+	if (name == NULL) {
+		name = "libmonodroid.so";
+		h = load_dso_from_override_dirs (name, dl_flags);


I feel like there are a lot of places in this PR where we call load_dso_from_override_dirs() then load_dso_from_app_lib_dirs() if load_dso_from_override_dirs() fails.

Perhaps we should have a helper which does that? load_dso(), perhaps? (Which admittedly already exists, but I'm seriously getting confused trying to keep all these new functions straight.)

No, this is done on purpose - load_dso loads a specific DSO, with a path that's already determined. This is the "core" loader. The override and app directory functions do the searching - I don't want to cram those together. This is a clear separation and on build time the calls will be optimized out again.

jonpryor · 2018-09-13T19:21:16Z

src/monodroid/jni/monodroid-glue.c

-	if (!h && name && (strstr (name, ".dll.so") || strstr (name, ".exe.so"))) {
-		char *full_name2;
-		const char *basename;
+	h = load_dso_from_override_dirs (name, dl_flags);


Like here again, we call load_dso_from_override_dirs() then call load_dso_from_app_lib_dirs() if the first call fails. This is a very common pattern; we should better support it.

jonpryor · 2018-09-13T19:22:31Z

src/monodroid/jni/monodroid-glue.c

-		log_info (LOG_ASSEMBLY, "Trying to load library '%s'", full_name);
-		free (full_name);
+	if (name == NULL) {
+		name = "libmonodroid.so";


I don't understand this if block. At a quick reading, it appears to set name="libmonodroid.so", then do...exactly what would be done when name != NULL. Why the duplication, instead of:

if (name == NULL) { name = "libmonodroid.so"; }

grendello · 2018-09-13T22:47:53Z

build

jonpryor · 2018-09-14T20:36:12Z

src/monodroid/jni/monodroid-glue.c

 static int android_api_level = 0;

+// Values correspond to the CPU_KIND_* macros
+static char* android_abi_names[CPU_KIND_X86_64+1] = {


I'm kinda surprised that not using const char doesn't result in a warning from the compiler. (Double negatives for the win?)

jonpryor · 2018-09-14T23:05:11Z

src/monodroid/jni/monodroid-glue.c

+			return;
+		log_info (LOG_BUNDLE, "Attempting to load bundled app from %s", bundle_path);
+		libapp = load_dso (bundle_path, dlopen_flags, TRUE);
+	}


Shouldn't we free (bundle_path); bundle_path = NULL; here?

jonpryor · 2018-09-14T23:05:49Z

src/monodroid/jni/monodroid-glue.c

+			log_fatal (LOG_BUNDLE, "bundled app initialization error");
+			exit (FATAL_EXIT_CANNOT_LOAD_BUNDLE);
+		} else {
+			log_info (LOG_BUNDLE, "bundled app not found in the APK, ignoring.");


Why should we ignore here and not exit? Why would embedded_dso_mode matter for this check?

Short version: if we exit, embedded DSO stops working.

Long version: in non-embedded mode the decision to set up a bundled app is based on the presence, on the filesystem, of the bundled app .so file - we stat the filesystem to make the decision. In embedded mode we cannot do the stat inside the zip/apk and so we always dlopen all the .so and see if that succeeded. Since it would always fail here, if the application wasn't a bundled one, the DSO mode would never work. As it currently stands, there's no other way to make this work except to ignore the failure to load the bundle DSO. It can (and should) be improved (as stated in the comment above the setup_bundled_app function) by allowing the code to detect if we're inside a bundled app by other means - for instance an environment variable added to the environment file when building the app. This change, however, is beyond scope of this PR.

jonpryor · 2018-09-14T23:06:07Z

src/monodroid/jni/monodroid-glue.c

+
+	mono_mkbundle_initialize_mono_api = dlsym (libapp, "initialize_mono_api");
+	if (!mono_mkbundle_initialize_mono_api)
+		log_error (LOG_BUNDLE, "Missing initialize_mono_api in the application");


Should this also exit the process?

This is another instance of not changing/fixing the existing logic/code. I agree we should exit, but it's not the topic of this PR.

jonpryor · 2018-09-14T23:06:22Z

src/monodroid/jni/monodroid-glue.c

+
+	mono_mkbundle_init = dlsym (libapp, "mono_mkbundle_init");
+	if (!mono_mkbundle_init)
+		log_error (LOG_BUNDLE, "Missing mono_mkbundle_init in the application");


Should this also exit the process?

jonpryor · 2018-09-15T00:30:08Z

src/monodroid/jni/monodroid-glue.c

+		handle = load_dso (full_path, dlopen_flags, FALSE);
+		free (full_path);
+		found = load_profiler_from_handle (handle, desc, mname);
+	};


Unnecessary ;.

jonpryor · 2018-09-15T00:33:52Z

src/monodroid/jni/monodroid-glue.c


 static void
-for_each_apk (JNIEnv *env, jobjectArray runtimeApks, void (*handler) (const char *apk, int index, int apk_count))
+for_each_apk (JNIEnv *env, jobjectArray runtimeApks, void *user_data, void (*handler) (const char *apk, int index, int apk_count, void *user_data))


Isn't the user_data parameter conventionally the last parameter, e.g. g_qsort_with_data(), g_list_foreach()?

Rephrased, I'd expect:

for_each_apk (JNIEnv *env, jobjectArray runtimeApks, void (*handler) (const char *apk, int index, int apk_count, void *user_data), void *user_data)

jonpryor · 2018-09-15T00:35:35Z

src/monodroid/jni/monodroid-glue.c

+static void
+add_apk_libdir (const char *apk, int index, int apk_count, void *user_data)
+{
+	if (user_data == NULL || index < 0 || index >= app_lib_directories_size)


...or should we just assert() these?

Context: dotnet#1906 Fixes: dotnet#1906 Android API 23 introduced a new way of dealing with the native shared libraries shipped in the APK. Before that API level, the libraries would be always extraced and placed in the application data directory, thus occupying more space than necessary. API 23 added a new manifest `<application>` element attribute, `android:extractNativeLibs`, which if set makes Android not extract the libraries to the filesystem. API 23 added a way to load those libraries directly from the APK. In order to support that there are a few requirements which this commit implements: * DSO (`.so`) files must be stored uncompressed in the APK * `<application android:extractNativeLibs="false"/>` must be present * DSOs in the APK must be aligned on the memory page boundary (the `-p` flag passed to `zipalign` takes care of that This commit also implements `libmonodroid` suport for loading our DSOs directly from the APK. This operation mode is enabled by the presence of the `__XA_DSO_IN_APK` environment variable. This variable is inserted into the application's environment by way of placing it in the environment file (a file part of the XA project that is marked with the `AndroidEnvironment` build action). In that mode, the DSOs are *no longer* looked up in the application data directory but only in the override directories (if the APK is built in Debug configuration) and in the APK itself. Currently, in order to activate the above mode, one has to perform the following actions manually: * Add the `android:extractNativeLibs="false"` attribute to the `<application>` element in the `Properties/AndroidManifest.xml` file * Add the following property to the project file: <AndroidStoreUncompressedFileExtensions>.so</AndroidStoreUncompressedFileExtensions> * Add an android environment file to the project with a line which says __XA_DSO_IN_APK=1 After that the application should work in the embedded DSO mode without problems. A couple of tests are provided to test building and execution of embedded DSO application on device, as well as to validate the built APK.

radekdoulik · 2018-09-18T09:56:01Z

src/monodroid/jni/dylib-mono.c

 }

-int monodroid_dylib_mono_init (struct DylibMono *mono_imports, const char *libmono_path)
+int monodroid_dylib_mono_init (struct DylibMono *mono_imports, void *libmono_handle)


Ops, this API change breaks Java.Interop here https://github.com/xamarin/java.interop/blob/master/src/java-interop/java-interop-gc-bridge-mono.c#L266

I can update Java.Interop. I wonder though, whether we consider libmonodroid API as public, because it can break someone else too?

When we migrate to C++ there will be more changes here. That code becomes a class and I added extern "C" versions of the functions - we can standarize/stabilize the API after the migration.

Context: dotnet#2154 Fixes: dotnet#2415 Currently, in order to activate the embedded DSO support, one has to perform the following actions manually: 1. Add the `android:extractNativeLibs="false"` attribute to the `<application>` element in the `Properties/AndroidManifest.xml` file. 2. Add the following property to the project file: <AndroidStoreUncompressedFileExtensions>.so</AndroidStoreUncompressedFileExtensions> 3. Add an android environment file to the project with a line which says: __XA_DSO_IN_APK=1 ~~ Changes ~~ The presence of `android:extractNativeLibs="false"` in `AndroidManifest.xml` should setup all the extra "stuff" so developers don't have to manually do it. `android:extractNativeLibs="false"` would automatically do the following: 1. `.so` will be appended to `$(AndroidStoreUncompressedFileExtensions)`. Both the `<Aapt/>` and `<BuildApk/>` MSBuild tasks use this property. 3. A new generated file in `$(IntermediateOutputPath)` will add `__XA_DSO_IN_APK=1` as an `@(AndroidEnvironment)` build item. I also added an MSBuild test to verify these changes are happening. I was also able to update `tests/EmbeddedDSOs` to rely on the new functionality.

Context: dotnet#2154 Fixes: dotnet#2415 Currently, in order to activate the embedded DSO support, one has to perform the following actions manually: 1. Add the `android:extractNativeLibs="false"` attribute to the `<application>` element in the `Properties/AndroidManifest.xml` file. 2. Add the following property to the project file: <AndroidStoreUncompressedFileExtensions>.so</AndroidStoreUncompressedFileExtensions> 3. Add an android environment file to the project with a line which says: __XA_DSO_IN_APK=1 ~~ Changes ~~ The presence of `android:extractNativeLibs="false"` in `AndroidManifest.xml` should setup all the extra "stuff" so developers don't have to manually do it. `android:extractNativeLibs="false"` would automatically do the following: 1. `.so` will be appended to `$(AndroidStoreUncompressedFileExtensions)`. Both the `<Aapt/>` and `<BuildApk/>` MSBuild tasks use this property. 3. A new generated file in `$(IntermediateOutputPath)` will add `__XA_DSO_IN_APK=1` as an `@(AndroidEnvironment)` build item. The problem here is with incremental builds... If `_GenerateJavaStubs` is skipped, we still need to know if `android:extractNativeLibs` is set. To make this work, I added a `CacheFile` property on the `GenerateJavaStubs` MSBuild task. It writes a simple XML document in `$(IntermediateOutputPath)javastubs.cache` such as: <Properties> <EmbeddedDSOsEnabled>True</EmbeddedDSOsEnabled> </Properties> The file will not exist at all unless the value is `True`, which will prevent this feature from impacting build times. I also added an MSBuild test to verify these changes are happening. I was also able to update `tests/EmbeddedDSOs` to rely on the new functionality. ~~ Other Changes ~~ I updated `_GenerateJavaStubs` to follow the "stamp" file convention of using `$(_AndroidStampDirectory)_GenerateJavaStubs.stamp`.

Fixes: #2415 Context: #2154 Currently, in order to activate the embedded Dynamic Shared Object (DSO) support, one has to perform the following actions manually (see also 95ca102): 1. Add the `android:extractNativeLibs="false"` attribute to the `<application>` element within `Properties/AndroidManifest.xml`. 2. Add the following property to the project file: <AndroidStoreUncompressedFileExtensions>.so</AndroidStoreUncompressedFileExtensions> 3. Add an android environment file to the project with a line containing: __XA_DSO_IN_APK=1 Instead of requiring these three separate steps, the presence of the `//application[@android:extractNativeLibs='false']` attribute within `AndroidManifest.xml` should instead be used as the "feature" toggle; when `extractNativeLibs` is false, then `$(AndroidStoreUncompressedFileExtensions)` should be automatically updated and the `__XA_DSO_IN_APK` environment variable should automatically exported: 1. Append the value `.so` to `$(AndroidStoreUncompressedFileExtensions)`. Both of the `<Aapt/>` and `<BuildApk/>` MSBuild tasks use this property. 2. Create a new file within `$(IntermediateOutputPath)` containing the string `__XA_DSO_IN_APK=1`, and add this new file to `@(AndroidEnvironment)` so that `__XA_DSO_IN_APK` is set. The problem here is with incremental builds; if the `_GenerateJavaStubs` target is skipped, we still need to know if `//application/@android:extractNativeLibs` is set. To make this work, I added a `CacheFile` property on the `<GenerateJavaStubs/>` MSBuild task. It writes a simple XML document to `$(IntermediateOutputPath)javastubs.cache`, such as: <Properties> <EmbeddedDSOsEnabled>True</EmbeddedDSOsEnabled> </Properties> The file will not exist at all unless the value is `True`, which will prevent this feature from impacting build times. I also added an MSBuild test to verify these changes are happening. I also updated `tests/EmbeddedDSOs` to rely on the new functionality. Finally, I updated the `_GenerateJavaStubs` target to follow the "stamp" file convention of using `$(_AndroidStampDirectory)_GenerateJavaStubs.stamp`.

grendello added the do-not-merge PR should not be merged. label Sep 6, 2018

grendello requested a review from jonpryor September 6, 2018 20:58

jonpryor reviewed Sep 7, 2018

View reviewed changes

src/monodroid/jni/monodroid-glue.c Show resolved Hide resolved

jonpryor reviewed Sep 7, 2018

View reviewed changes

radekdoulik reviewed Sep 7, 2018

View reviewed changes

grendello force-pushed the issue1906 branch from 5574528 to 9530269 Compare September 7, 2018 06:55

grendello requested a review from dellis1972 as a code owner September 7, 2018 10:36

grendello force-pushed the issue1906 branch 6 times, most recently from dd03d38 to 29ecb9e Compare September 10, 2018 19:04

grendello added the run-tests-release label Sep 11, 2018

grendello force-pushed the issue1906 branch from 29ecb9e to b35465d Compare September 12, 2018 18:17

grendello force-pushed the issue1906 branch from b35465d to 9393936 Compare September 13, 2018 14:25

grendello removed the do-not-merge PR should not be merged. label Sep 13, 2018

grendello force-pushed the issue1906 branch from 9393936 to ab053de Compare September 13, 2018 14:27

jonpryor reviewed Sep 13, 2018

View reviewed changes

src/monodroid/jni/monodroid-glue.c Show resolved Hide resolved

jonpryor reviewed Sep 13, 2018

View reviewed changes

grendello force-pushed the issue1906 branch 2 times, most recently from 9cc55ca to ecee315 Compare September 13, 2018 21:58

grendello changed the title ~~[WIP] First steps to support DSOs embedded in the .apk~~ First steps to support DSOs embedded in the .apk Sep 13, 2018

grendello changed the title ~~First steps to support DSOs embedded in the .apk~~ Support DSOs embedded in the .apk Sep 13, 2018

grendello force-pushed the issue1906 branch 2 times, most recently from 0f1e436 to 23edd75 Compare September 14, 2018 09:24

grendello mentioned this pull request Sep 14, 2018

monodroid.so can't find other mono .so libraries in Android Things OEM application #1906

Closed

jonpryor reviewed Sep 14, 2018

View reviewed changes

jonpryor reviewed Sep 15, 2018

View reviewed changes

grendello force-pushed the issue1906 branch from 23edd75 to 853c7c5 Compare September 15, 2018 09:05

EmbeddedDSOs should always run.

8a2ca61

jonpryor merged commit 95ca102 into dotnet:master Sep 17, 2018

grendello deleted the issue1906 branch September 17, 2018 18:23

radekdoulik reviewed Sep 18, 2018

View reviewed changes

jonathanpeppers mentioned this pull request Jan 2, 2019

[Xamarin.Android.Build.Tasks] support Embedded DSOs #2579

Merged

github-actions bot locked and limited conversation to collaborators Feb 2, 2024

Support DSOs embedded in the .apk #2154

Support DSOs embedded in the .apk #2154

Uh oh!

Conversation

grendello commented Sep 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grendello commented Sep 7, 2018

Uh oh!

grendello commented Sep 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonpryor Sep 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grendello commented Sep 13, 2018

Uh oh!

Choose a reason for hiding this comment

grendello commented Sep 6, 2018 •

edited

Loading

jonpryor Sep 14, 2018 •

edited

Loading