Skip to content

Conversation

jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Feb 27, 2025

Context: dotnet/runtime#112109
Context: f5ce0ad
Context: 3824b97

Commit f5ce0ad copied portions of the .NET for Android Mono GC Bridge
code into src/java-interop, but to build that code "on Desktop" --
for possible CI and unit test use -- it needed to build against Mono
somehow.

The actual "how" was unspecified:

  • The macOS build implicitly required that
    /Library/Frameworks/Mono.framework exist, and would pull in
    header files and libraries from there.

  • Linux and Windows were not support at all.

Eventually src/java-interop got Windows build support in 3824b97.
This allowed Java.Interop-Tests.dll to run under .NET Core, but
this did not contain any GC bridge code; it was just the JNI
wrapper functions such as java_interop_jnienv_get_version() and
JVM loading functions such as java_interop_jvm_create().

Eventually the Mono GC bridge code effectively bitrotted, as nothing
built it anymore. .NET for Android has its own (original!) copy,
and CI is primarily concerned about .NET (Core) and not Mono.

Three things have happened in the meantime:

  1. The Mono team was absorbed into the .NET team, with the new
    "MonoVM" as a supported backend for .NET.
    MonoVM is used by .NET for Android and a supported backend with
    .NET for iOS, Mac, etc.

  2. MonoVM is now provided as NuGet packages!

  3. There are now efforts to investigate adding something like the GC
    Bridge to CoreCLR and NativeAOT; see GC: Support object level interop on Android runtime#112109.
    Some developers involved with this would like to debug how the
    current GC bridge works. On desktop Windows.

Embrace the new world order: remove support for using "system" Mono
installations, the _CreateMonoInfoProps target, generation of
MonoInfo.props, etc., and instead use MonoVM packages to provide
the MonoVM runtime and header files needed for compilation & linking.
This allows for a consistent build environment across Linux, macOS,
and Windows, though Windows adds one wrinkle:

MonoVM doesn't contain a coreclr.lib file for linking.

Add coreclr.def and a coreclr.lib file to use for linking against
MonoVM on Windows. coreclr.lib was created by:

lib /def:coreclr.def /out:coreclr.lib /machine:X64

where coreclr.def contains the mono_ symbols that were referenced
by the macOS libjava-interop.dylib build, as per
nm bin/Release-net8.0/libjava-interop.dylib | grep 'U _mono'.

Update src/java-interop so that it builds with the latest MonoVM
headers (<mono/metadata/appdomain.h> must now be included), and
allow it to build under Windows (use Interlocked* instead of
clang's __sync_*_and_fetch().)

There is one ("tiny") problem with this approach: .NET 9 doesn't
contain MonoVM packages. For example, the
Microsoft.NETCore.App.Runtime.Mono.osx-x64 NuGet package was
last updated for .NET 9 Preview 7; the last stable version is for
.NET 8.

Fortunately we only recently migrated to .NET 9 (f30e420).
Update JniRuntime.cs so that we can build the repo when overriding
with -p:DotNetTargetFramework=net8.0.

Update Java.Interop.Sdk so that the .jar it creates is
appropriately copied to $(OutputPath) of the referencing project.
This required using %(Content.CopyToOutputDirectory)=PreserveNewest
alongside using %(Content.TargetPath) -- which sets the relative
name within $(OutputPath) -- and in turn required making
_JavaCreateJcws run after CoreCompile instead of Build, so that
it would properly participate in %(CopyToOutputDirectory) logic.

The code to detect that MonoVM is being used has changed; the
Mono.Runtime type no longer exists. Mono.RuntimeStructs needs
to be checked for instead.

Update TestJVM.GetJvmLibraryPath() so that it can find
bin\BuildRelease\JdkInfo.props when running an app from the
samples directory and not from bin\TestRelease*\….

All of this allows for samples/Hello-Java.Base to be built against
and run with MonoVM, on Windows!

# need Release config build because `dotnet publish` is implicitly Release
dotnet build -c Release -t:Prepare -p:DotNetTargetFramework=net8.0 Java.Interop.sln
dotnet build -c Release -p:DotNetTargetFramework=net8.0 Java.Interop.sln

# -p:UseMonoRuntime enables use of MonoVM
dotnet publish --self-contained -p:UseMonoRuntime=true -p:DotNetTargetFramework=net8.0 -p:UseAppHost=true -p:ErrorOnDuplicatePublishOutputFiles=false samples/Hello-Java.Base/Hello-Java.Base.csproj -r win-x64

Note: replace -r win-x64 with the appropriate RID for your host.

With that setup, you can then run the sample:

> samples\Hello-Java.Base\bin\Release\win-x64\publish\Hello-Java.Base.exe
MonoVM support enabled
# jonp: LoadJvmLibrary(C:\Users\jopryo\android-toolchain\jdk-17\bin\server\jvm.dll)=140710450692096
# jonp: JNI_CreateJavaVM=140710454831248; JNI_GetCreatedJavaVMs=140710454831280
# jonp: executing JNI_CreateJavaVM=7ff9b4ad2890
# jonp: r=0 javavm=7ff9b51fd7f0 jnienv=24d781dddc0
WARNING in native method: JNI call made without checking exceptions when required to from CallStaticObjectMethodV
binding? net.dot.jni.sample.MyJLO@a09ee92

jonpryor and others added 8 commits February 27, 2025 17:59
Insert description here.

`src/java-interop` support for building against MonoVM was broken
at some point, and is arguably still *wrong* because even now it's
using the legacy Mono header files, *not* the current MonoVM headers.

TODO: figure out how to fix that.

*For now*, fix `_CreateMonoInfoProps` so that
`bin/Build$(Configuration)/MonoInfo.props` is created, which in turn
allows `src/java-interop` ***on macOS and Linux*** to use the
mono headers and link against libmonosgen-2.0.dylib/etc.:

	% nm src/java-interop/obj/Debug-net8.0/libjava-interop.dylib | grep _mono
	                 …
	                 U _mono_class_from_mono_type

Currently unknown: how do we *use* `libjava-interop.dylib`?

The 'obvious' way is to build a project with `-p:UseMonoRuntime=true`.
That fails under .NET 9:

	error NU1102: Unable to find package Microsoft.NETCore.App.Runtime.Mono.osx-x64 with version (= 9.0.2)

Try to re-add support for building with .NET 8, by overriding
`$(DotNetTargetFramework)`, which *does* build with `-p:UseMonoRuntime=true`:

```
dotnet build -c Release -t:Prepare -p:DotNetTargetFramework=net8.0 *.sln
dotnet build -c Release -p:DotNetTargetFramework=net8.0 *.sln

dotnet publish --self-contained -p:UseMonoRuntime=true -p:DotNetTargetFramework=net8.0 -p:UseAppHost=true -p:ErrorOnDuplicatePublishOutputFiles=false -r osx-x64 samples/Hello-Java.Base/Hello-Java.Base.csproj

cp samples/Hello-Java.Base/bin/Release/Hello-Java.Base.jar samples/Hello-Java.Base/bin/Release/osx-x64/publish

samples/Hello-Java.Base/bin/Release/osx-x64/publish/Hello-Java.Base
```

Update `JreRuntime.cs` to check for the existence of `Mono.RuntimeStructs`,
as `Mono.Runtime` no longer exists.

I don't yet know if this uses the GC bridge, though.
We should use the MonoVM packages instead.
At this point, *on macOS*, the following commands result in a build
of `samples/Hello-Java.Base` which use MonoVM + the Mono GC bridge:

    # prepare and build the repo against .NET 8, as that has the needed MonoVM packages
    dotnet build -c Release -t:Prepare -p:DotNetTargetFramework=net8.0 *.sln
    dotnet build -c Release -p:DotNetTargetFramework=net8.0 *.sln

    # Publish the Hello-Java.Base sample; must be `--self-contained` + `-p:UseMonoRuntime=true`
    # to get the MonoVM
    dotnet publish -c Release --self-contained -p:UseMonoRuntime=true -p:DotNetTargetFramework=net8.0 -p:UseAppHost=true -p:ErrorOnDuplicatePublishOutputFiles=false -r osx-x64 samples/Hello-Java.Base/Hello-Java.Base.csproj

    # Something is missing in the targets that `Hello-Java.Base.jar` isn't copied; copy it.
    # TODO: fix this.
    cp samples/Hello-Java.Base/bin/Release/Hello-Java.Base.jar samples/Hello-Java.Base/bin/Release/osx-x64/publish

With that setup, you can then run the sample:

    % samples/Hello-Java.Base/bin/Release/osx-x64/publish/Hello-Java.Base
    MonoVM support enabled
    # jonp: LoadJvmLibrary(/Library/Java/JavaVirtualMachines/microsoft-17.jdk/Contents/Home/lib/libjli.dylib)=1164832048
    # jonp: JNI_CreateJavaVM=4572626688; JNI_GetCreatedJavaVMs=4572626768
    # jonp: executing JNI_CreateJavaVM=1108cbf00
    # jonp: r=0 javavm=11272e690 jnienv=7fe0a32e42a8
    WARNING in native method: JNI call made without checking exceptions when required to from CallStaticObjectMethodV
    binding? net.dot.jni.sample.MyJLO@70dea4e

Next step: how much of this works targeting win-x64?
It breaks the Windows build; `publish Hello-NativeAOTFromJNI`
fails to build with:

    C:\hostedtoolcache\windows\dotnet\sdk\9.0.200\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.targets(266,5): error NETSDK1047: Assets file 'D:\a\1\s\src\java-interop\obj\project.assets.json' doesn't have a target for 'net9.0/osx-x64'. Ensure that restore has run and that you have included 'net9.0' in the TargetFrameworks for your project. You may also need to include 'osx-x64' in your project's RuntimeIdentifiers. [D:\a\1\s\src\java-interop\java-interop.csproj]

Perhaps Don't Do That™?
Means I don't need to worry about 32-bit vs. 64-bit `coreclr.lib` files.

With this change, the following (terrible hack) "works":

```
dotnet build -c Release -t:Prepare -p:DotNetTargetFramework=net8.0 Java.Interop.sln
dotnet build -c Release -p:DotNetTargetFramework=net8.0 Java.Interop.sln

dotnet publish --self-contained -p:UseMonoRuntime=true -p:DotNetTargetFramework=net8.0 -p:UseAppHost=true -p:ErrorOnDuplicatePublishOutputFiles=false -r win-x64 samples/Hello-Java.Base/Hello-Java.Base.csproj

copy samples\Hello-Java.Base\bin\Release\Hello-Java.Base.jar samples\Hello-Java.Base\bin\Release\win-x64

dotnet build -c Release -p:DotNetTargetFramework=net8.0 -r win-x64 src\java-interop\java-interop.csproj
copy bin\Release-net8.0\win-x64\win-x64 samples\Hello-Java.Base\bin\Release\win-x64
```

...until you try to *run* the app, then it fails:

	> samples\Hello-Java.Base\bin\Release\win-x64\Hello-Java.Base.exe
	MonoVM support enabled
	# jonp: LoadJvmLibrary(C:\Users\jopryo\android-toolchain\jdk-1.8\jre\bin\server\jvm.dll)=1344667648
	# jonp: JNI_CreateJavaVM=1345963424; JNI_GetCreatedJavaVMs=1345963728
	# jonp: executing JNI_CreateJavaVM=5039c5a0
	# jonp: r=0 javavm=509e0650 jnienv=13e1d0b6a58
	WARNING in native method: JNI call made without checking exceptions when required to from CallStaticObjectMethodV
	WARNING in native method: JNI call made with exception pending
	WARNING in native method: JNI call made with exception pending

	Unhandled Exception:
	System.TypeInitializationException: The type initializer for 'Types' threw an exception.
	 ---> System.ArgumentNullException: Value cannot be null. (Parameter 'method')
	   at Java.Interop.JniEnvironment.InstanceMethods.CallObjectMethod(JniObjectReference instance, JniMethodInfo method) in Q:\src\dotnet\java-interop\src\Java.Interop\obj\Release\net8.0\JniEnvironment.g.cs:line 19974

...but that's because it's using *JDK 1.8*.

Tell it to use JDK 11+ (17+?), and it works!

	>samples\Hello-Java.Base\bin\Release\win-x64\Hello-Java.Base.exe --jvm C:\Users\jopryo\android-toolchain\jdk-17\bin\server\jvm.dll
	MonoVM support enabled
	# jonp: LoadJvmLibrary(C:\Users\jopryo\android-toolchain\jdk-17\bin\server\jvm.dll)=140710737870848
	# jonp: JNI_CreateJavaVM=140710742010000; JNI_GetCreatedJavaVMs=140710742010032
	# jonp: executing JNI_CreateJavaVM=7ff9c5cb2890
	# jonp: r=0 javavm=7ff9c63dd7f0 jnienv=15f85f0a0e0
	WARNING in native method: JNI call made without checking exceptions when required to from CallStaticObjectMethodV
	binding? net.dot.jni.sample.MyJLO@a09ee92
@jonpryor jonpryor force-pushed the dev/jonp/jonp-resuscitate-mono-gc-bridge branch from 299ac75 to 224433e Compare March 3, 2025 21:43
jonpryor and others added 10 commits March 5, 2025 15:31
Context: 37f68a3

Commit 37f68a3 contained a comment+TODO:

	# Something is missing in the targets that `Hello-Java.Base.jar` isn't copied; copy it.
	# TODO: fix this.
	cp samples/Hello-Java.Base/bin/Release/Hello-Java.Base.jar samples/Hello-Java.Base/bin/Release/osx-x64/publish

Fix that.  The extra `cp` is no longer needed.

Fixing this required three things:

 1. Adding the JCW `.jar` to `@(Content)` (or `@(None)`, or…)
 2. Setting `%(Content.CopyToOutputDirectory)`=PreserveNewest
 3. Setting `%(Content.TargetPath)`

`%(TargetPath)` is used to control the filename within the final
`$(OutputPath)` directory tree; *without* `%(TargetPath)`, then
`%(Identity)` is used, and since `%(Identity)` is now within
`$(IntermediateOutputPath)`, that's *wrong*.

Rename `$(JavaOutputJar)` to `$(JavaOutputJarName)`, as the
new implied semantic is that it *cannot* contain a directory; it's
*just* the filename.

Update the `_JavaCreateJcws` so that instead of being a post-Build
target it's instead a post-CoreCompile target.  This is required
because post-Build is too late; MSBuild tries to copy the `@(Content)`
entries to `$(OutputPath)` *before the end* of `Build`, and as a
post-Build step it didn't have a chance to *create* the jar yet!
A post-CoreCompile target works nicer in this context.

Because we're now post-CoreCompile, `$(TargetPath)` doesn't exist.
(That's created during the build, by copying the intermediate
assembly to the outptu path.)  Update `_JavaCreateJcws` to use
`@(IntermediateAssembly)` instead of `$(TargetPath)`.

Update unit tests accordingly.
Context: 1a5093b
Context: e056bc0

Building upon 1a5093b: yes, let's just use MonoVM!
…but the problem with is the use of `$(RuntimeIdentifier)`, which
isn't provided by default.

Simplify things by *not* worrying about/supporting cross-compilation
-- we don't need to support building win-x64 from osx-x64 --
*and* dropping support for win-x86 (64-bit for everyone! Plus, this
is just to support debugging anyway, NOT for product).

Which in turn means we can drop the `@(_JavaInteropNativeLib)`
item group (we don't need two Windows entries anymore).

This in turn allows us to take a page from e056bc0, and put the
`$(IntermediateOutputPath)`-containing native library into
`@(None)`, with `%(CopyToOutputDirectory)`=PreserveNewest.
This in turn means that *referencing projects* such as
`samples/Hello-Java.Base` will have it properly copied to their
`$(OutputPath)`.  Which means the (incomplete, doh!) `copy`
mentioned in 224433e is no longer needed.
Context: 224433e

Remember 224433e?

> ...until you try to *run* the app, then it fails:
>
>         > samples\Hello-Java.Base\bin\Release\win-x64\Hello-Java.Base.exe
>         MonoVM support enabled
>         # jonp: LoadJvmLibrary(C:\Users\jopryo\android-toolchain\jdk-1.8\jre\bin\server\jvm.dll)=1344667648
>         # jonp: JNI_CreateJavaVM=1345963424; JNI_GetCreatedJavaVMs=1345963728
>         # jonp: executing JNI_CreateJavaVM=5039c5a0
>         # jonp: r=0 javavm=509e0650 jnienv=13e1d0b6a58
>         WARNING in native method: JNI call made without checking exceptions when required to from CallStaticObjectMethodV
>         WARNING in native method: JNI call made with exception pending
>         WARNING in native method: JNI call made with exception pending
>
>         Unhandled Exception:
>         System.TypeInitializationException: The type initializer for 'Types' threw an exception.
>
> ...but that's because it's using *JDK 1.8*.
>
> Tell it to use JDK 11+ (17+?), and it works!
>
>         >samples\Hello-Java.Base\bin\Release\win-x64\Hello-Java.Base.exe --jvm C:\Users\jopryo\android-toolchain\jdk-17\bin\server\jvm.dll
>         MonoVM support enabled
>         # jonp: LoadJvmLibrary(C:\Users\jopryo\android-toolchain\jdk-17\bin\server\jvm.dll)=140710737870848
>         # jonp: JNI_CreateJavaVM=140710742010000; JNI_GetCreatedJavaVMs=140710742010032
>         # jonp: executing JNI_CreateJavaVM=7ff9c5cb2890
>         # jonp: r=0 javavm=7ff9c63dd7f0 jnienv=15f85f0a0e0
>         WARNING in native method: JNI call made without checking exceptions when required to from CallStaticObjectMethodV
>         binding? net.dot.jni.sample.MyJLO@a09ee92

Update `TestJVM.GetJvmLibraryPath()` to probe for more locations.
It was previously assuming that the assembly would be located within
`bin/Tests*`, which isn't the case for the sample.  It now will find
`<checkout>\bin\Build{Debug,Release}\JdkInfo.props`, avoiding the need
to provide the `--jvm=PATH` argument.

Additionally, `java-interop.targets` was occasionally failing to build
from `samples\Hello-Java.Base` because it was trying to build
`libjava-interop.dylib` for arm64 on my x86 machine, which is not
supported.  (Not sure why this didn't break before.)
We need jnienv-gen.csproj to be built before java-interop.csproj, but
if `_BuildLibs` happens *before* ResolveReferences, that doesn't happen,
and thus the build fails.

Update _BuildLibs to happen after ResolveReferences, which ensures that
jnienv-gen.csproj is built first.
    CMake Warning (dev) at CMakeLists.txt:1 (project):
      cmake_minimum_required() should be called prior to this top-level project()
      call.  Please see the cmake-commands(7) manual for usage documentation of
      both commands.
    This warning is for project developers.  Use -Wno-dev to suppress it.

Enable diagnostic verbosity to figure out why things aren't building.
`-v:diag` looks reasonable, in that `$(_MonoNativePath)` contained a
value which looked like what I expected.

Yet the build still fails:

    In file included from /Users/runner/work/1/s/src/java-interop/java-interop-gc-bridge-mono.cc:10: (TaskId:621)
    /Users/runner/work/1/s/src/java-interop/java-interop-mono.h:6:10: fatal error: 'mono/metadata/assembly.h' file not found (TaskId:621)
    #include <mono/metadata/assembly.h> (TaskId:621)
             ^~~~~~~~~~~~~~~~~~~~~~~~~~ (TaskId:621)
     (TaskId:621)

I must assume that while the properties have expected values, the
*contents on disk* are…missing?  Or something?

Start logging what's there.
Maybe?

Reviewing the build log of 2f1a127
shows that MonoVM bits were *not* present, which explains a bit.

But why?

*Probably* because of the Condition around `@(PackageDownload)`, which
required that `$(RuntimeIdentifier)` be set.

It isn't set.

Nor was `$(UseMonoRuntime)`; therefore `@(PackageDownload)` was empty,
and MonoVM bits were not downloaded.

Additionally, the diag build log from 3d41a11
shows a new-to-me `$(NETCoreSdkRuntimeIdentifier)` property.

Remove use of `$(_RuntimeIdentifier)` and use `$(NETCoreSdkRuntimeIdentifier)`.
@jonpryor jonpryor marked this pull request as ready for review March 6, 2025 19:34
@jonpryor
Copy link
Contributor Author

jonpryor commented Mar 6, 2025

Draft commit message:

Context: https://github.com/dotnet/runtime/issues/112109
Context: f5ce0ad22b3adb6ccdd5f8587eab310d415e1f16
Context: 3824b974dada6f302ed5f7b7f97ffbd08990a82a

Commit f5ce0ad2 copied portions of the .NET for Android Mono GC Bridge
code into `src/java-interop`, but to *build* that code "on Desktop" --
for possible CI and  unit test use -- it needed to build against Mono
*somehow*.

The *actual* "how" was unspecified:

  * The macOS build implicitly required that
    `/Library/Frameworks/Mono.framework` exist, and would pull in
    header files and libraries from there.

  * Linux and Windows were not support *at all*.

Eventually `src/java-interop` got Windows build support in 3824b974.
This allowed `Java.Interop-Tests.dll` to run under .NET Core, but
this did *not* contain any GC bridge code; it was *just* the JNI
wrapper functions such as `java_interop_jnienv_get_version()` and
JVM loading functions such as `java_interop_jvm_create()`.

Eventually the Mono GC bridge code effectively bitrotted, as nothing
*built* it anymore.  .NET for Android has its own (original!) copy,
and CI is primarily concerned about .NET (Core) and not Mono.

Three things have happened in the meantime:

 1. The Mono team was absorbed into the .NET team, with the new
    "MonoVM" as a supported backend for .NET.
    MonoVM is used by .NET for Android and a supported backend with
    .NET for iOS, Mac, etc.

 2. MonoVM is now provided as NuGet packages!

 3. There are now efforts to investigate adding something like the GC
    Bridge to CoreCLR and NativeAOT; see dotnet/runtime#112109.
    Some developers involved with this would like to debug how the
    current GC bridge works.  *On desktop Windows*.

Embrace the new world order: remove support for using "system" Mono
installations, the `_CreateMonoInfoProps` target, generation of
`MonoInfo.props`, etc., and instead use MonoVM packages to provide
the MonoVM runtime and header files needed for compilation & linking.
This allows for a consistent build environment across Linux, macOS,
and Windows, though Windows adds one wrinkle:

MonoVM doesn't contain a `coreclr.lib` file for linking.

Add `coreclr.def` and a `coreclr.lib` file to use for linking against
MonoVM on Windows.  `coreclr.lib` was created by:

	lib /def:coreclr.def /out:coreclr.lib /machine:X64

where `coreclr.def` contains the `mono_` symbols that were referenced
by the macOS `libjava-interop.dylib` build, as per
`nm bin/Release-net8.0/libjava-interop.dylib | grep 'U _mono'`.

Update `src/java-interop` so that it builds with the latest MonoVM
headers (`<mono/metadata/appdomain.h>` must now be included), and
allow it to build under Windows (use `Interlocked*` instead of
clang's `__sync_*_and_fetch()`.)

There is one ("tiny") problem with this approach: .NET 9 doesn't
contain MonoVM packages.  For example, the
[`Microsoft.NETCore.App.Runtime.Mono.osx-x64` NuGet package][0] was
last updated for .NET 9 Preview 7; the last *stable* version is for
.NET *8*.

Fortunately we only recently migrated to .NET 9 (f30e420a).
Update `JniRuntime.cs` so that we can build the repo when overriding
with `-p:DotNetTargetFramework=net8.0`.

Update `Java.Interop.Sdk` so that the `.jar` it creates is
appropriately copied to `$(OutputPath)` of the referencing project.
This required using `%(Content.CopyToOutputDirectory)`=PreserveNewest
alongside  using `%(Content.TargetPath)` -- which sets the relative
name within `$(OutputPath)` -- and in turn required making
`_JavaCreateJcws` run after `CoreCompile` instead of `Build`, so that
it would properly participate in `%(CopyToOutputDirectory)` logic.

The code to detect that MonoVM is being used has changed; the
`Mono.Runtime` type no longer exists.  `Mono.RuntimeStructs` needs
to be checked for instead.

Update `TestJVM.GetJvmLibraryPath()` so that it can find
`bin\BuildRelease\JdkInfo.props` when running an app from the
`samples` directory and not from `bin\TestRelease*\…`.

All of this allows for `samples/Hello-Java.Base` to be built against
and run with MonoVM, on Windows!

	# need Release config build because `dotnet publish` is implicitly Release
	dotnet build -c Release -t:Prepare -p:DotNetTargetFramework=net8.0 Java.Interop.sln
	dotnet build -c Release -p:DotNetTargetFramework=net8.0 Java.Interop.sln

	# -p:UseMonoRuntime enables use of MonoVM
	dotnet publish --self-contained -p:UseMonoRuntime=true -p:DotNetTargetFramework=net8.0 -p:UseAppHost=true -p:ErrorOnDuplicatePublishOutputFiles=false samples/Hello-Java.Base/Hello-Java.Base.csproj -r win-x64

Note: replace `-r win-x64` with the appropriate RID for your host.

With that setup, you can then run the sample:
	
	> samples\Hello-Java.Base\bin\Release\win-x64\publish\Hello-Java.Base.exe
	MonoVM support enabled
	# jonp: LoadJvmLibrary(C:\Users\jopryo\android-toolchain\jdk-17\bin\server\jvm.dll)=140710450692096
	# jonp: JNI_CreateJavaVM=140710454831248; JNI_GetCreatedJavaVMs=140710454831280
	# jonp: executing JNI_CreateJavaVM=7ff9b4ad2890
	# jonp: r=0 javavm=7ff9b51fd7f0 jnienv=24d781dddc0
	WARNING in native method: JNI call made without checking exceptions when required to from CallStaticObjectMethodV
	binding? net.dot.jni.sample.MyJLO@a09ee92

[0]: https://www.nuget.org/packages/Microsoft.NETCore.App.Runtime.Mono.osx-x64/

@jonpryor jonpryor requested a review from Copilot March 6, 2025 20:41
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR updates the Java interop code to resuscitate the Mono GC Bridge by switching from relying on system Mono installations to using MonoVM NuGet packages. Key changes include:

  • Adding optional logging support for JVM path probing in tests.
  • Updating the Mono detection mechanism and logging in the runtime.
  • Adjusting build and runtime behavior for better cross-platform compatibility.

Reviewed Changes

File Description
tests/TestJVM/TestJVM.cs Added logger parameters to several methods, and introduced a new probing strategy for JdkInfo.props.
samples/Hello-Java.Base/Program.cs Updated JVM path handling and added console logger creation based on verbosity.
src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs Changed Mono detection from "Mono.Runtime" to "Mono.RuntimeStructs" and logged MonoVM support.
src/Java.Interop/Java.Interop/JniRuntime.cs Wrapped a break call with a NET9_0_OR_GREATER conditional compilation directive.

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

tests/TestJVM/TestJVM.cs:121

  • Accessing jdkJvmPath.Value without a null-check can lead to a NullReferenceException if the XML element is missing. Consider verifying that jdkJvmPath is not null before using its Value.
logger?.Invoke (TraceLevel.Verbose, $"TestJVM: $(JavaSdkDirectory)={jdkPath?.Value}; $(JdkJvmPath)={jdkJvmPath.Value}");

Comment on lines +32 to 35
<PropertyGroup Condition=" $([MSBuild]::IsOSPlatform ('osx')) ">
<_CmakeOsxArch Condition=" '$(NETCoreSdkRuntimeIdentifier)' == 'osx-x64' ">x86_64</_CmakeOsxArch>
<_CmakeOsxArch Condition=" '$(NETCoreSdkRuntimeIdentifier)' == 'osx-arm64' ">arm64</_CmakeOsxArch>
</PropertyGroup>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth making it error on linux? Or can we guess what the paths might be there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are values provided to the Cmake CMAKE_OSX_ARCHITECTURES variable. $(CmakeOsxArch) is not used for Linux, and would make no sense there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two sections in this file for: $([MSBuild]::IsOSPlatform ('osx')) and $([MSBuild]::IsOSPlatform ('windows')), so it just made me think the linux one is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, but currently I don't think anything is missing for Linux.

(Note: I haven't actually built Linux yet, so Linux is basically a thought experiment until someone does so. 😉)

<MakeDir Directories="$(IntermediateOutputPath)%(_JavaInteropNativeLib.Dir)" />
DependsOnTargets="GetNativeBuildCommands;_BuildJni_c;_GetJavaInteropBuildInfo"
AfterTargets="ResolveReferences"
Inputs="CMakeLists.txt;$(MSBuildThisFileFullPath);java-interop.csproj;@(ClInclude);@(ClCompile)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the other files inputs like coreclr.def, coreclr.lib?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand the question.

Conceptually, coreclr.lib could be added as an input, but (1) it should rarely change, and (2) if it does need to change, it's because you have a build failure anyway, e.g. because we added a call to a new mono_whatever() function that wasn't previously listed in coreclr.def/coreclr.lib.

It feels moot to add them, unless we add a Target which generates coreclr.lib from coreclr.def

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the question is if coreclr.def/coreclr.lib change, do you want cmake to run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they change, it's because cmake errored out, in which case the Outputs won't exist, in which case updating coreclr.* won't matter, because re-building will re-run cmake anyway.

The only scenario this won't be the case is if you build successfully, then update the .lib to remove used symbols, and build again…

in which case, yeah, coreclr.lib should be an Input. Doh!

Context: #1316 (comment)

Also, add a `_UpdateCoreCLRLib` target to codify the command used
to generate coreclr.lib from coreclr.def.
@jonpryor jonpryor merged commit a5b229d into main Mar 7, 2025
2 checks passed
@jonpryor jonpryor deleted the dev/jonp/jonp-resuscitate-mono-gc-bridge branch March 7, 2025 16:13
jonpryor pushed a commit to dotnet/android that referenced this pull request Mar 10, 2025
Changes: dotnet/java-interop@6096f8b...93c3872

  * dotnet/java-interop@93c38729: [Hello-Java.Base] MonoVM + peer collection (dotnet/java-interop#1319)
  * dotnet/java-interop@a5b229da: [java-interop] Resuscitate MonoVM GC Bridge (dotnet/java-interop#1316)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants