-
Notifications
You must be signed in to change notification settings - Fork 545
[Mono.Android] call new Java "GC Bridge" APIs #10125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
bbc1c64
to
58739d3
Compare
} | ||
} | ||
return peers; | ||
} | ||
} | ||
|
||
static GCHandle CreateReferenceTrackingHandle (IJavaPeerable value) => | ||
JavaMarshal.CreateReferenceTrackingHandle (value, value.PeerReference.Handle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This concerns me, because this value will change over time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To elaborate, with MonoVM -- and thus I assume eventually with dotnet/runtime#114184 -- when a "bridged" instance is no longer reachable from the .NET GC, we:
- "Toggle"
JavaObject.PeerReference
from a JNI Global Reference to a JNI Weak Global Reference. - Trigger a Java-side GC.
- If the instance survives the Java-side GC, we "toggle" the JNI Weak Global Reference to a JNI Global Reference.
This means the value of value.PeerReference
changes, because 1...3 will result in a different JNI Global Reference value.
Meaning using "value.PeerReference.Handle` is immediately "bad".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also wonder if using value.PeerReference
is necessary? It's for the userContext
parameter; perhaps it could just be null
?
E.g. CreateCrossReferenceHandle()
calls CreateHandleWithExtraInfo()
and providing userContext
as the "extra info", and JavaMarshal.GetContext()
returns GetExtraInfoFromHandle()
, i.e. appears to return "userContext".
Aside: if true, this makes me think that JavaMarshal.GetContext()
is mis-named, and should instead be called JavaMarshal.GetUserContext()
, which would make the relationship more apparent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…though, to be fair, it's CreateReferenceTrackingHandle(object obj, System.IntPtr context)
, so GetContext()
does make sense…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where @jonpryor got to in the end is how this should be used. He is correct in that the PeerReference
(assumed to be a JNI handle) should be contained within the allocated memory that is the "context". Renaming to "UserContext" is a reasonable request and we can workshop that in API review.
The PeerReference
property could dereference the context pointer and get the JNI handle if desired. In fact, the context could be a pointer sized allocation or any size, store any addtiional details you want there.
Context: dotnet/runtime#114184 Context: https://github.com/jonathanpeppers/BridgeSandbox Context: dotnet/runtime@main...BrzVlad:runtime:feature-clr-gcbridge So far, I: * Built dotnet/runtime, I built this branch: https://github.com/jonathanpeppers/runtime/tree/gcbridge_impl * Put the relevant arm64 packs in `packages/` folder and configured a `NuGet.config` to use them. * Setup `build-tools\scripts\custom-runtime.targets` to use the 10.0.0-dev dotnet/runtime packs. * In `ManagedValueManager.cs`... * Call a new native method on startup: var mark_cross_references_ftn = RuntimeNativeMethods.clr_initialize_gc_bridge (&BridgeProcessingFinished); * Pass the returned function pointer to: JavaMarshal.Initialize (mark_cross_references_ftn); * In `AddPeer(IJavaPeerable value)` call `JavaMarshal.CreateReferenceTrackingHandle()` * In `RemovePeer(IJavaPeerable value)` call: static unsafe void FreeHandle (GCHandle handle) { IntPtr context = JavaMarshal.GetContext (handle); NativeMemory.Free((void*)context); }
ab51219
to
f4a2148
Compare
What I suspect needs to happen is that partial class JavaObject {
// Remove the following members
[NonSerialized] IntPtr handle;
[NonSerialized] JniObjectReferenceType handle_type;
// Used by JavaInteropGCBridge
[NonSerialized] IntPtr weak_handle;
[NonSerialized] int refs_added;
} and replace with: internal struct JniObjectInfo {
public IntPtr handle;
public JniObjectReferenceType handle_type;
// Used by JavaInteropGCBridge
public IntPtr weak_handle;
public int refs_added;
}
partial class JavaObject {
IntPtr JniObjectInfo = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(JniObjectInfo)));
} …then update everything within This would allow Further note: this in turn means that the appropriate starting point for this PR is in dotnet/java-interop, not dotnet/android. |
The new APIs are all decorated with:
We can use them in java.interop, but would have to wrap them all with java.interop is also not building with The new APIs will only be in .NET 10. So, maybe we have to do a branch of java.interop that just updates |
[UnmanagedCallersOnly] | ||
internal static unsafe void FinishBridgeProcessing (nint sccsLen, StronglyConnectedComponent* sccs, nint ccrsLen, ComponentCrossReference* ccrs) | ||
{ | ||
Java.Lang.JavaSystem.Gc (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the C# binding of java.lang.System.gc()
: https://developer.android.com/reference/java/lang/System#gc()
Context: dotnet/runtime#114184
Context: https://github.com/jonathanpeppers/BridgeSandbox
Context: dotnet/runtime@main...BrzVlad:runtime:feature-clr-gcbridge
So far, I:
https://github.com/jonathanpeppers/runtime/tree/gcbridge_impl
Put the relevant arm64 packs in
packages/
folder and configured aNuGet.config
to use them.Setup
build-tools\scripts\custom-runtime.targets
to use the10.0.0-dev dotnet/runtime packs.
In
ManagedValueManager.cs
...Call a new native method on startup:
In
AddPeer(IJavaPeerable value)
callJavaMarshal.CreateReferenceTrackingHandle()
In
RemovePeer(IJavaPeerable value)
call:Update
I added a second commit, of what it could look like if everything was managed/C# code.