[libjava-interop] Use own DylibMono instance

[radekdoulik]

The monodroid's DylibMono is initialized in
Java_mono_android_Runtime_init and thus not avaiable for
standalone Java.Interop consumers, like jnimarshalmethod-gen.exe.

Let libjava-interop have its own DylibMono instance and initialize
it on first call to mono functions.

We might rethink that once we start using libjava-interop's GC bridge
in monodroid.

[jonpryor]

I'm not sure I understand this change or the need for this specific change.

As per the earlier comment, you're going to update Java.Runtime.Environment.dll.config so that it uses e.g. libmono-android.debug.dylib instead of libjava-interop.dylib, right?

This might work on macOS because monodroid_dylib_mono_init(&mono, NULL) will eventually call dlopen(NULL), and since libjava-interop.dylib contains a link reference to libmonosgen-2.0.dylib, this will work, as mono is already loaded into the process because java-interop was loaded.

This will fail on Windows, because libmono-android.debug.dll will not contain a library link reference to libmonosgen-2.0.dll, it's all handled dynamically.

I believe what we need to do is make src/java-interop like xamarin-android/src/monodroid:

1. Move dylib-mono.* from xamarin-android into Java.Interop.
2. "Plumb" the Java.Interop APIs so that the path to libmono can be provided at runtime. For example, this might be JreRuntimeOptions.MonoLibraryPath property, which can be specified at runtime via e.g. jnimarshalmethod-gen.exe -mono-path=/path/to/libmonosgen-2.0.dylib.

That way, things at least have a chance of working on Windows, and we'll make the java-interop test environment closer-in-spirit to what xamarin-android does.

[radekdoulik]

OK, I wasn't aware about the situation on Windows, so I expected monodroid_dylib_mono_init(&mono, NULL) to work happily there.

[radekdoulik]

As per the earlier comment, you're going to update Java.Runtime.Environment.dll.config so that it uses e.g. libmono-android.debug.dylib instead of libjava-interop.dylib, right?

Yes, plus we need to add Java.Interop.dll.config as well. (for https://github.com/xamarin/java.interop/blob/master/src/Java.Interop/Java.Interop/JniEnvironment.cs#L168)

[radekdoulik]

Thinking more about it, I think on Windows it should work as well. We will be running jnimarshalmethod-gen.exe with mono (so that dll.config files with dllmap elements will work). In that case the libmonosgen should be already loaded and RTLD_DEFAULT should be OK then.

[jonpryor]

Will RTLD_DEFAULT be OK? We should test that.

[radekdoulik]

Tried to test it on Windows. Looks like we might indeed need to specify the mono runtime library path. It is not completely clear, but likely. The mono is crashing silently and the debug output looks incomplete.

End of mono -v jnimarshalmethod-gen.exe -v -f --keep-temp ..\jni-test\xat\xatemplateaot.dll output:

...
Method void Java.Interop.MonoRuntimeValueManager:OnSetRuntime (Java.Interop.JniRuntime) emitted at 00000259C9170290 to 00000259C9170753 (code length 1219) [workspace]
converting method void Java.Interop.JniRuntime/JniValueManager:OnSetRuntime (Java.Interop.JniRuntime)
Method void Java.Interop.JniRuntime/JniValueManager:OnSetRuntime (Java.Interop.JniRuntime) emitted at 00000259C91707F0 to 00000259C9170888 (code length 152) [workspace]
converting method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_get_current ()
Method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_get_current () emitted at 00000259C91708A0 to 00000259C917097D (code length 221) [workspace]
converting method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_new (intptr)
Method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_new (intptr) emitted at 00000259C9170990 to 00000259C9170A75 (code length 229) [workspace]
converting method System.RuntimeTypeHandle System.RuntimeType:get_TypeHandle ()
Method System.RuntimeTypeH

When I tried to pass the mono library path, it got a bit further and crashed silently again. The end of output of updated run:

...
Method void Java.Interop.JniRuntime/JniValueManager:OnSetRuntime (Java.Interop.JniRuntime) emitted at 00000177203C06E0 to 00000177203C0778 (code length 152) [workspace]
converting method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_get_current ()
Method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_get_current () emitted at 00000177203C0790 to 00000177203C086D (code length 221) [workspace]
converting method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_new (intptr)
Method (wrapper managed-to-native) intptr Java.Interop.NativeMethods:java_interop_gc_bridge_new (intptr) emitted at 00000177203C0880 to 00000177203C0965 (code length 229) [workspace]
converting method System.RuntimeTypeHandle System.RuntimeType:get_TypeHandle ()
Method System.RuntimeTypeHandle System.RuntimeType:get_TypeHandle () emitted at 00000177203C0980 to 00000177203C09DB (code length 91) [workspace]
converting method (wrapper managed-to-native) int Java.Interop.NativeMethods:java_interop_gc_bridge_set_bridge_processing_field (intptr,System.RuntimeTypeHandle,string)
Method (wrapper managed-to-native) int Java.Interop.NativeMethods:java_interop_gc_bridge_set_bridge_processing_field (intptr,System.RuntimeTypeHandle,string) emitted at 00000177203C09F0 to 00000177203C0B1D (code length 301) [workspace]
...
Method (wrapper managed-to-native) void System.Threading.ThreadPool:InitializeVMTp (bool&) emitted at 00000177203C1480 to 00000177203C1555 (code length 213) [workspace]
converting method void System.Threading.QueueUserWorkItemCallback:.cctor ()
Method void System.Threading.QueueUserWorkItemCallback:.cctor () emitted at 00000177203C1580 to 00000177203C15FB (code length 123) [workspace]
converting method void System.Threading.QueueUserWorkItemCallback:.ctor (System.Threading.WaitCallback,object,bool,System.Threading.StackCrawlMark&)
Method void System.Threading.QueueUserWorkItemCallback:.ctor (System.Threading.WaitCallback,object,bool,System.Threading.StackC

So it looks like we will have to fix more issues on Windows.

• above mentioned crash
• jvm.dll location on Windows to add to Java.Runtime.Environement.dll.config (reuse or move the JdkInfo task from XA?)
• include mono.exe binary in the XA installation on Windows as well?

Therefore I would like to merge this PR as is, so that I can finish the new methods marshaling on mac and then get back and fix remaining issues on Windows.

Sounds good?

[jonpryor]

I still don't understand why this patch is needed.

src/java-interop/java-interop-mono.h has a #include "monodroid-glue.h":

https://github.com/xamarin/java.interop/blob/bed29582923e35e692ae9354404937df6c5c1863/src/java-interop/java-interop-mono.h#L9

monodroid-glue.h declares monodroid_get_dylib():

https://github.com/xamarin/xamarin-android/blob/32adcc6926435455305aa65af8d271504f6e2cc5/src/monodroid/jni/monodroid-glue.h#L13

It seems that the primary purpose here is to call monodroid_dylib_mono_init(), which is currently only called from Java_mono_android_Runtime_init().

If that's the actual problem, couldn't we instead just call monodroid_dylib_mono_init() from "somewhere early" to ensure that monodroid_get_dylib() works?

We know that mid-term monodroid_dylib_mono_init (&mono, NULL) will not work (Windows), so if we need a "hack" for short-term purposes, why not a small & directed "hack" that explicitly calls monodroid_dylib_mono_init()?

[radekdoulik]

It seems that the primary purpose here is to call monodroid_dylib_mono_init(), which is currently only called from Java_mono_android_Runtime_init().

That is right.

We know that mid-term monodroid_dylib_mono_init (&mono, NULL) will not work (Windows), so if we need a "hack" for short-term purposes, why not a small & directed "hack" that explicitly calls monodroid_dylib_mono_init()?

That would work too. I considered that before, but because didn't think of it as a hack at that time, I didn't find good place to do it.

Updated the patch and used beginning of java_interop_gc_bridge_new to initialize it.

[jonpryor]

...how does this build and run on the PR builder?

That this works at all without #if is blowing my mind.

Looking at the build log, we're seeing the warning I would expect to see:

java-interop-gc-bridge-mono.c(265,7): warning G84A3CD4A: implicit declaration of function 'monodroid_dylib_mono_init' is invalid in C99 [-Wimplicit-function-declaration] [/Users/builder/jenkins/workspace/Java.Interop-pr-builder/src/java-interop/java-interop.csproj]
          if (!monodroid_dylib_mono_init (monodroid_get_dylib (), NULL)) {
               ^

Good thing we don't have "warning as errors" enabled! ;-)

Reading further, my mind is no longer blown: it's blowing up during unit tests:

  mono --debug packages/NUnit.ConsoleRunner.3.7.0/tools/nunit3-console.exe  bin/TestDebug/Java.Interop-Tests.dll  --result="TestResult-Java.Interop-Tests.xml;format=nunit2" --output="bin/TestDebug/TestOutput-Java.Interop-Tests.txt"
...
  Test Files
      bin/TestDebug/Java.Interop-Tests.dll
  
  dyld: lazy symbol binding failed: Symbol not found: _monodroid_get_dylib
    Referenced from: /Users/builder/jenkins/workspace/Java.Interop-pr-builder/bin/TestDebug/libjava-interop.dylib
    Expected in: flat namespace
  
  dyld: Symbol not found: _monodroid_get_dylib
    Referenced from: /Users/builder/jenkins/workspace/Java.Interop-pr-builder/bin/TestDebug/libjava-interop.dylib
...
/Users/builder/jenkins/workspace/Java.Interop-pr-builder/build-tools/scripts/RunNUnitTests.targets(30,5): error MSB3073: The command "mono --debug packages/NUnit.ConsoleRunner.3.7.0/tools/nunit3-console.exe  bin/TestDebug/Java.Interop-Tests.dll  --result="TestResult-Java.Interop-Tests.xml;format=nunit2" --output="bin/TestDebug/TestOutput-Java.Interop-Tests.txt"" exited with code 156.
  Build continuing because "ContinueOnError" on the task "Exec" is set to "ErrorAndContinue".
Done Building Project "/Users/builder/jenkins/workspace/Java.Interop-pr-builder/build-tools/scripts/RunNUnitTests.targets" (default targets) -- FAILED.

Build FAILED.

Given that it clearly failed...WHY DIDN'T IT FAIL?!

Now my mind is blown, just differently from before. :-(

With that exploration out of the way...

[jonpryor]

This if block needs to be wrapped in #if defined (ANDROID) || defined (DYLIB_MONO):

#if defined (ANDROID) || defined (DYLIB_MONO)
	if (!monodroid_dylib_mono_init (monodroid_get_dylib (), NULL)) {
		log_fatal (LOG_DEFAULT, "mono runtime initialization error: %s", dlerror ());
		exit (FATAL_EXIT_CANNOT_FIND_MONO);
	}
#endif  /* defined (ANDROID) || defined (DYLIB_MONO) */

[radekdoulik]

Doh, indeed. At least we have the PR builder fixed as well. Good catch!

[jonpryor]

hopefully I've fixed the PR builder, so...

[jonpryor]

build