-
Notifications
You must be signed in to change notification settings - Fork 54
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
Reference Counting, non-bridged backend #426
Comments
Fixes: dotnet#426 Enable C#8 [Nullable Reference Types][0] for `Java.Runtime.Environment.dll`. Add support for a "non-bridged backend", so that a `JniRuntime.JniValueManager` exists for .NET Core. This new "managed" backed is used if the Mono runtime is *not* used. To work, `ManagedValueManager` holds *strong* references to `IJavaPeerable` instances. As such, tests which required the use of GC integration are now "optional". To make this work, update `JniRuntime.JniValueManager` to have the following new abstract members: partial class JniRuntime { partial class JniValueManager { public abstract bool PeersRequireRelease {get;} public abstract void ReleasePeers (); } } `PeersRequireRelease` shall be `true` when there is no GC bridge. When this is true, `JniValueManager.CollectPeers()` is a no-op. If an `IJavaPeerable` must have disposal logic performed, then `.Dispose()` must be called on that instance. There is no finalization integration. The new `JniValueManager.ReleasePeers()` method: 1. Releases all GREFs for all held peers. This allows Java to collect the Java peers. 2. Stops referencing all `IJavaPeerable` values. This allows the .NET GC to collect the `IJavaPeerable` values. There is no notification to the `IJavaPeerable` instances that this has happened. Update `Java.Interop-Tests.csproj` to define `NO_MARSHAL_MEMBER_BUILDER_SUPPORT` when building for .NET Core. These changes allow all remaining `Java.Interop-Tests` unit tests to execute under .NET Core: dotnet test -v diag '--logger:trx;verbosity=detailed' bin/TestDebug-netcoreapp3.1/Java.Interop-Tests.dll Other changes: * The attempt to retain useful Java-side exceptions in 89a5a22 proved to be incomplete. Add a comment to invoke [`JNIEnv::ExceptionDescribe()`][1]. We don't always want this to be present, but when we do want it… * While `NO_MARSHAL_MEMBER_BUILDER_SUPPORT` is set -- which means that `Java.Interop.Export`-related tests aren't run -- there are some fixes for `Java.Interop.Export` & related unit tests for .NET Core, to avoid the use of generic delegate types and to avoid a `Type.GetType()` which is no longer needed. [0]: https://docs.microsoft.com/dotnet/csharp/nullable-references [1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#ExceptionDescribe
After…checks notes…almost two years of thought, I think I was wrong on the need for all the restrictions. Related: PR #804. The fundamental question/scenario is around object lifetimes, especially since there are two "peers" that both need to be kept alive: the Java peer and the Managed peer. In Mono/Xamarin.Android, Mono's SGen Bridge is used, which allows us to "work with" Mono's GC. Each Managed peer holds a JNI Global Reference to the Java peer, keeping the Java peer alive, and the GC bridge is used to ensure that the Managed peer is kept alive so long as managed code holds a reference or so long as something in Java-land holds a reference to the Java peer. This allows the lifetimes of the two peers to be closely correlated. Without a GC bridge, we can instead have stricter semantics:
This prevents objects from being prematurely collected. It also prevents any objects from ever being collected by the GC. For that "minor" problem, all of the "meanings" implied in the initial issue are no longer implied. (Insert hand-waving here.)
The (large!) remaining problem is one of coordination: eventually you want the objects to be collected. The only way to do so is to "dispose" of the Managed peer, which will free the Java peer. However, the Java peer may still be referenced by other Java code, and if that Java code invokes any methods on the (now disconnected) Java peer, those methods will fail. Thus the only "safe" way to fully "dispose" of a Managed peer is to ensure no Java code is referencing it. …which is hard to do without a GC bridge. (Baby steps?) A place where this may still be viable is if "higher level" code can ensure that once "something is done", all the Java & Managed objects are no longer relevant, and thus can be freed. This remains a "non-ideal" scenario, but it is sufficient to get most of the Java.Interop unit tests executing… |
To help "support"/"address" the coordination problem, PR #804 adds two members to partial class JniRuntime {
partial class JniValueManager {
public abstract bool PeersRequireRelease {get;}
public abstract void ReleasePeers ();
}
} The The There is a broader question of semantics: should There are currently two "lifetime" methods:
See e.g. In a CoreCLR environment, Question: Should |
Fixes: dotnet#426 Enable C#8 [Nullable Reference Types][0] for `Java.Runtime.Environment.dll`. Add support for a "non-bridged backend", so that a `JniRuntime.JniValueManager` exists for .NET Core. This new "managed" backed is used if the Mono runtime is *not* used. To work, `ManagedValueManager` holds *strong* references to `IJavaPeerable` instances. As such, tests which required the use of GC integration are now "optional". To make this work, update `JniRuntime.JniValueManager` to have the following new abstract members: partial class JniRuntime { partial class JniValueManager { public abstract bool PeersRequireRelease {get;} public abstract void ReleasePeers (); } } `PeersRequireRelease` shall be `true` when there is no GC bridge. When this is true, `JniValueManager.CollectPeers()` is a no-op. If an `IJavaPeerable` must have disposal logic performed, then `.Dispose()` must be called on that instance. There is no finalization integration. The new `JniValueManager.ReleasePeers()` method: 1. Releases all GREFs for all held peers. This allows Java to collect the Java peers. 2. Stops referencing all `IJavaPeerable` values. This allows the .NET GC to collect the `IJavaPeerable` values. There is no notification to the `IJavaPeerable` instances that this has happened. Update `Java.Interop-Tests.csproj` to define `NO_MARSHAL_MEMBER_BUILDER_SUPPORT` when building for .NET Core. These changes allow all remaining `Java.Interop-Tests` unit tests to execute under .NET Core: dotnet test -v diag '--logger:trx;verbosity=detailed' bin/TestDebug-netcoreapp3.1/Java.Interop-Tests.dll Other changes: * The attempt to retain useful Java-side exceptions in 89a5a22 proved to be incomplete. Add a comment to invoke [`JNIEnv::ExceptionDescribe()`][1]. We don't always want this to be present, but when we do want it… * While `NO_MARSHAL_MEMBER_BUILDER_SUPPORT` is set -- which means that `Java.Interop.Export`-related tests aren't run -- there are some fixes for `Java.Interop.Export` & related unit tests for .NET Core, to avoid the use of generic delegate types and to avoid a `Type.GetType()` which is no longer needed. [0]: https://docs.microsoft.com/dotnet/csharp/nullable-references [1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#ExceptionDescribe
Possibly related? #4 The idea in Issue #4 is that sometimes methods return the "same" value: // Java
public sealed class Example {
private Example e = new Example();
public static Example getSingleton() {
return e;
}
} We'd bind this kinda/sorta like: // C# binding
[Register (…)\
public sealed class Example {
static readonly JniPeerMembers _members = new JniPeerMembers ("…/Example", typeof (Example));
public static Example Singleton {
[Register (…)]
get {
const string __id = "getSingleton.()L…/Example;";
var __rm = _members.StaticMethods.InvokeObjectMethod (__id, null);
return JniEnvironment.Runtime.ValueManager.GetValue<Example>(__rm, JniObjectReferenceOptions.CopyAndDispose);
}
} Due to our "identity preserving semantics", that means that the same instance is returned by var a = Example.Singleton;
var b = Example.Singleton;
if (object.ReferenceEquals (a, b)) … // always true However, it's not possible to know, in all circumstances, whether a "new value" or a "previously registered value" will be returned. Consequently, if you want to keep GREF use low, this won't work: // DO NOT DO
using (var a = Example.Singleton) {
// …
} The above won't work in multithreaded environments, because if another thread B also accesses The current "solution" is, in effect, Don't Do That™: don't use The proposed solution was to alter the "single registered instance" behavior to permit possible "aliasing", but this time in a good way! using (var scope = JniEnvironment.BeginGetValueBehaviors (GetValueBehaviors.CreateValues)
using (var a = Example.Singleton) {
// …
} This could be thread-safe, as |
Proposal: Instead of public enum JniValueManagerCollectsRegistrations {
Never,
WithSystemGC,
}
partial class JniValueManager {
public abstract JniValueManagerCollectsRegistrations JniValueManagerCollectsRegistrations {get;}
public abstract IDisposable CreateRegistrationScope (…);
} The using (var scope = JniEnvironment.Runtime.ValueManager.CreateRegistrationScope (…)) {
// create new `JavaObject` subclass instances
}
// all instances created within `scope` are Disposed Such scopes would need to support being thread-local, in order to avoid cross-thread sharing. There also needs to be support for a "global" scope, to assist with the "Tensorflow+Java"-esque scenario: if other threads are created, and you're at "steady state", you may need to clear everything. This suggests: [Flags]
public enum RegistrationScope {
None,
Global = 1 << 0,
ThreadLocal = 1 << 1,
Dispose = 1 << 4,
Release = 1 << 5,
}
Only one of Still not sure adding a new "global" scope necessarily makes sense; how's that work in a multi-threaded scenario? Since a "scope" can't reasonably "partially nest", the entire May also want/need to update public partial enum JniObjectReferenceOptions {
CopyAndRegisterGlobal = Copy + (1 << 3),
} Current |
Design thoughts… Instead of a
Additionally, A scenario behind #426 was e.g. to do a "C# plugin for a machine learning pipeline": have Tensorflow+Java installed on a server, write a plugin for it within C#, and then process lots of images in a pipeline with the C# plugin. In such an environment, the normal GC bridge isn't necessarily required: between each image you can "release everything" and do some GC's so that you don't leak. My suspicion, though, is that you wouldn't want to release everything. You'd just want to release "everything created since some well-defined point". This would allow one to initialize things once, get a good "starting state", and then process the pipeline, without needn't to re-initialize between each item.
I think we need a better abstraction than what is proposed here. Probably something "stack-like". |
Related related idea? Model on Objective-C AutoRelease pools? https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmAutoreleasePools.html The problem here is that we don't have reference counting, so it's a bit of an "all or nothing" thing. |
Fixes: dotnet#426 Enable C#8 [Nullable Reference Types][0] for `Java.Runtime.Environment.dll`. Add support for a "non-bridged backend", so that a `JniRuntime.JniValueManager` exists for .NET Core. This new "managed" backed is used if the Mono runtime is *not* used. To work, `ManagedValueManager` holds *strong* references to `IJavaPeerable` instances. As such, tests which required the use of GC integration are now "optional". To make this work, update `JniRuntime.JniValueManager` to have the following new abstract members: partial class JniRuntime { partial class JniValueManager { public abstract bool PeersRequireRelease {get;} public abstract void ReleasePeers (); } } `PeersRequireRelease` shall be `true` when there is no GC bridge. When this is true, `JniValueManager.CollectPeers()` is a no-op. If an `IJavaPeerable` must have disposal logic performed, then `.Dispose()` must be called on that instance. There is no finalization integration. The new `JniValueManager.ReleasePeers()` method: 1. Releases all GREFs for all held peers. This allows Java to collect the Java peers. 2. Stops referencing all `IJavaPeerable` values. This allows the .NET GC to collect the `IJavaPeerable` values. There is no notification to the `IJavaPeerable` instances that this has happened. Update `Java.Interop-Tests.csproj` to define `NO_MARSHAL_MEMBER_BUILDER_SUPPORT` when building for .NET Core. These changes allow all remaining `Java.Interop-Tests` unit tests to execute under .NET Core: dotnet test -v diag '--logger:trx;verbosity=detailed' bin/TestDebug-netcoreapp3.1/Java.Interop-Tests.dll Other changes: * The attempt to retain useful Java-side exceptions in 89a5a22 proved to be incomplete. Add a comment to invoke [`JNIEnv::ExceptionDescribe()`][1]. We don't always want this to be present, but when we do want it… * While `NO_MARSHAL_MEMBER_BUILDER_SUPPORT` is set -- which means that `Java.Interop.Export`-related tests aren't run -- there are some fixes for `Java.Interop.Export` & related unit tests for .NET Core, to avoid the use of generic delegate types and to avoid a `Type.GetType()` which is no longer needed. [0]: https://docs.microsoft.com/dotnet/csharp/nullable-references [1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#ExceptionDescribe
Having slept on it, wrt previous design thoughts… We don't want to support explicitly registering "global" instances, at least not at first. It may be useful, but without a concrete scenario in mind, it's extra complexity. Additionally, if the intent is something "reasonably easily used" for scoping, a [Flags]
public enum JniPeerRegistrationScopeOptions {
None,
Register = (1 << 1),
DoNotRegister = (1 << 2),
}
public enum JniPeerRegistrationScopeCleanup {
Dispose,
Release,
}
public sealed class JniPeerRegistrationScope : IDisposable {
public JniPeerRegistrationScope (JniPeerRegistrationScopeOptions options = JniPeerRegistrationScopeOptions.Register, JniPeerRegistrationScopeCleanup cleanup = JniPeerRegistrationScopeCleanup.Dispose);
public void Dispose ();
} This would allow "reasonable" (?) using (var scope = new JniPeerRegistrationScope()) {
var list = new Java.Util.ArrayList ();
}
// at scope.Dispose(), `list` and everything it contains is automatically .Dispose()d.
This in turn begs the question, how do we ensure it's thread-local? The answer here may be to be instead use a Pity However, public ref struct JniPeerRegistrationScope {
public JniPeerRegistrationScope (JniPeerRegistrationScopeOptions options, JniPeerRegistrationScopeCleanup cleanup = JniPeerRegistrationScopeCleanup.Dispose);
public void Dispose ();
} On the enum design, I'm still not thrilled with multiple enums, but I'm likewise not thrilled with a single enum containing distinct "sections", a'la public struct JniPeerRegistrationScopeOptions {
public bool DisposePeers {get; set;}
public bool ReleasePeers {get; set;}
public bool DoNotRegisterPeers {get; set;}
}
public ref struct JniPeerRegistrationScope {
public JniPeerRegistrationScope (JniPeerRegistrationScopeOptions);
public void Dispose ();
} |
…and more thinking, the current Instead, to support Issue #4, we'd want a I'm increasingly thinking that A thread-local "scope stack" should be part of internal partial class JniEnvironmentInfo {
public Stack<IDictionary<int, List<IJavaPeerable>> RegistrationScopes {get;}
}
public partial class JniEnvironment {
public IDictionary<int, List<IJavaPeerable>>? CurrentRegistrationScope => {
if (CurrentInfo.RegistrationScopes.TryPeek (out var scope))
return scope;
return null;
}
} |
"Related" random thought: there is a
If we had a |
Back on the partial class JniRuntime.JniValueManager {
public virtual bool CanCollectPeers => false;
public virtual void CollectPeers() => throw new NotSupportedException();
public abstract void ReleasePeers();
} This preserves the existing semantic that
Documentation-wise -- once we have docs -- we should also mention how |
Fixes: dotnet#4 Context: dotnet#426 Alternate names? * JavaPeerableCleanupPool * JavaPeerCleanup * JavaReferenceCleanup Issue dotnet#426 is an idea to implement a *non*-GC-Bridged `JniRuntime.JniValueManager` type, primarily for use with .NET Core. This was begun in a666a6f. What's missing is the answer to a question: what do do about `JniRuntime.JniValueManager.CollectPeers()`? With a Mono-style GC bridge, `CollectPeers()` is `GC.Collect()`. In a666a6f with .NET Core, `CollectPeers()` calls `IJavaPeerable.Dispose()` on all registered instances, which is "extreme". @jonpryor thought that if there were a *scope-based* way to selectively control which instances were disposed, that might be "better" and more understandable. Plus, this is issue dotnet#4! Add `JniPeerRegistrationScope`, which introduces a scope-based mechanism to control when `IJavaPeerable` instances are cleaned up: public enum JniPeerRegistrationScopeCleanup { RegisterWithManager, Dispose, Release, } public ref struct JniPeerRegistrationScope { public JniPeerRegistrationScope(JniPeerRegistrationScopeCleanup cleanup); public void Dispose(); } `JniPeerRegistrationScope` is a [`ref struct`][0], which means it can only be allocated on the runtime stack, ensuring that cleanup semantics are *scope* semantics. TODO: is that actually a good idea? If a `JniPeerRegistrationScope` is created using `JniPeerRegistrationScopeCleanup.RegisterWithManager`, existing behavior is followed. This is useful for nested scopes, should instances need to be registered with `JniRuntime.ValueManager`. If a `JniPeerRegistrationScope` is created using `.Dispose` or `.Release`, then: 1. Object references created within the scope are "thread-local"; they can be *used* by other threads, but `JniRuntime.JniValueManager.PeekPeer()` won't find existing values. 2. At the end of a `using` block / when `JniPeerRegistrationScope.Dispose()` is called, all collected instances will be `Dispose()`d (with `.Dispose`) or released (with `.Release`), and left to the GC to eventually finalize. For example: using (new JniPeerRegistrationScope (JniPeerRegistrationScopeCleanup.Dispose)) { var singleton = JavaSingleton.Singleton; // use singleton } // `singleton.Dispose()` is called at the end of the `using` block TODO: docs? TODO: *nested* scopes, and "bound" vs. "unbound" instance construction around `JniValueManager.GetValue<T>()` or `.CreateValue<T>()`, and *why* they should be treated differently. TODO: Should `CreateValue<T>()` be *removed*? name implies it always "creates" a new value, but implementation will return existing instances, so `GetValue<T>()` alone may be better. One related difference is that` `CreateValue<T>()` uses `PeekBoxedObject()`, while `GetValue<T>()` doesn't. *Should* it? [0]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/struct#ref-struct
Fixes: dotnet#4 Context: dotnet#426 Alternate names? * JavaPeerableCleanupPool * JavaPeerCleanup * JavaReferenceCleanup * JniPeerRegistrationScope Issue dotnet#426 is an idea to implement a *non*-GC-Bridged `JniRuntime.JniValueManager` type, primarily for use with .NET Core. This was begun in a666a6f. What's missing is the answer to a question: what do do about `JniRuntime.JniValueManager.CollectPeers()`? With a Mono-style GC bridge, `CollectPeers()` is `GC.Collect()`. In a666a6f with .NET Core, `CollectPeers()` calls `IJavaPeerable.Dispose()` on all registered instances, which is "extreme". @jonpryor thought that if there were a *scope-based* way to selectively control which instances were disposed, that might be "better" and more understandable. Plus, this is issue dotnet#4! Add `JavaScope`, which introduces a scope-based mechanism to control when `IJavaPeerable` instances are cleaned up: public enum JavaScopeCleanup { RegisterWithManager, Dispose, Release, } public ref struct JavaScope { public JavaScope(JavaScopeCleanup cleanup); public void Dispose(); } `JavaScope` is a [`ref struct`][0], which means it can only be allocated on the runtime stack, ensuring that cleanup semantics are *scope* semantics. TODO: is that actually a good idea? If a `JavaScope` is created using `JavaScopeCleanup.RegisterWithManager`, existing behavior is followed. This is useful for nested scopes, should instances need to be registered with `JniRuntime.ValueManager`. If a `JavaScope` is created using `JavaScopeCleanup.Dispose` or `JavaScopeCleanup.Release`, then: 1. Object references created within the scope are "thread-local"; they can be *used* by other threads, but `JniRuntime.JniValueManager.PeekPeer()` won't find existing values. 2. At the end of a `using` block / when `JavaScope.Dispose()` is called, all collected instances will be `Dispose()`d (with `.Dispose`) or released (with `.Release`), and left to the GC to eventually finalize. For example: using (new JavaScope (JavaScopeCleanup.Dispose)) { var singleton = JavaSingleton.Singleton; // use singleton } // `singleton.Dispose()` is called at the end of the `using` block TODO: docs? TODO: *nested* scopes, and "bound" vs. "unbound" instance construction around `JniValueManager.GetValue<T>()` or `.CreateValue<T>()`, and *why* they should be treated differently. TODO: Should `CreateValue<T>()` be *removed*? name implies it always "creates" a new value, but implementation will return existing instances, so `GetValue<T>()` alone may be better. One related difference is that` `CreateValue<T>()` uses `PeekBoxedObject()`, while `GetValue<T>()` doesn't. *Should* it? [0]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/struct#ref-struct
A possible alternate/additional idea suggested by
What's a "dependent handle"? So long as the key is kept alive, the value is kept alive. A problem with this is that it still appears to require that all keys be in the managed VM. How do we have JVM-hosted Java instances keep managed instances alive then? They're in separate VMs! However, that might not be as problematic as it first appears? Conceptually, we could introduce a internal partial class ManagedHandle {
public int Id {get;}
private ManagedHandle() {}
static List<ManagedHandle> handles = new List<ManagedHandle>();
public static ManagedHandle Alloc()
{
var h = new ManagedHandle();
handles.Add(h);
return h;
}
public static void Free(ManagedHandle h)
{
handles.Remove (h);
}
public static void Free(int id)
{
handles.RemoveAll (v => v.Id == id);
}
} We can then have a
// Java
/* partial */ class ManagedHandle {
public ManagedHandle();
@Override protected void finalize() { Runtime.freeManagedHandle(id); }
} Because of The setup: class MyObject : Java.Lang.Object {}
var o = new MyObject(); A problem here is that in "steady state" nothing can be collected: managed Here, we could have More pondering required. |
Fixes: #4 Context: #426 Context: a666a6f Alternate names? * JavaScope * JavaPeerableCleanupPool * JavaPeerCleanup * JavaReferenceCleanup * JniPeerRegistrationScope Issue #426 is an idea to implement a *non*-GC-Bridged `JniRuntime.JniValueManager` type, primarily for use with .NET Core. This was begun in a666a6f. What's missing is the answer to a question: what to do about `JniRuntime.JniValueManager.CollectPeers()`? With a Mono-style GC bridge, `CollectPeers()` is `GC.Collect()`. In a666a6f with .NET Core, `CollectPeers()` calls `IJavaPeerable.Dispose()` on all registered instances, which is "extreme". @jonpryor thought that if there were a *scope-based* way to selectively control which instances were disposed, that might be "better" and more understandable. Plus, this is issue #4! Which brings us to the background for Issue #4, which touches upon [bugzilla 25443][0] and [Google issue 37034307][1]: Java.Interop attempts to provide referential identity for Java objects: if two separate Java methods invocations return the same Java instance, then the bindings of those methods should also return the same instance. This is "fine" on the surface, but has a few related issues: 1. JNI Global References are a limited resource: Android only allows ~52000 JNI Global References to exist, which sounds like a lot, but might not be. 2. Because of (1), it is not uncommon to want to use `using` blocks to invoke `IJavaPeerable.Dispose()` to release JNI Global References. 3. However, because of object identity, you could unexpectedly introduce *cross-thread sharing* of `IJavaPeerable` instances. This might not be at all obvious; for example, in the Android 5 timeframe, [`Typeface.create()`][2] wouldn't necessarily return new Java instances, but may instead return cached instances. Meaning that this: var t1 = new Thread(() => { using var typeface = Typeface.Create("Family", …); // use typeface… }); var t2 = new Thread(() => { using var typeface = Typeface.Create("Family", …); // use typeface… }); t1.Start(); t2.Start(); t1.Join(); t2.Join(); could plausibly result in `ObjectDisposedException`s (or worse), as the threads could be unexpectedly sharing the same bound instance. Which *really* means that you can't reliably call `Dispose()`, unless you *know* you created that instance: using var safe = new Java.Lang.Double(42.0); // I created this, therefore I control all access and can Dispose() it using var unsafe = Java.Lang.Double.ValueOf(42.0); // I have no idea who else may be using this instance! Attempt to address both of these scenarios -- a modicum of .NET Core support, and additional sanity around JNI Global Reference lifetimes -- by introducing a new `JavaPeerableRegistrationScope` type, which introduces a scope-based mechanism to control when `IJavaPeerable` instances are cleaned up: public enum JavaPeerableRegistrationScopeCleanup { RegisterWithManager, Dispose, Release, } public ref struct JavaPeerableRegistrationScope { public JavaPeerableRegistrationScope(JavaPeerableRegistrationScopeCleanup cleanup); public void Dispose(); } `JavaPeerableRegistrationScope` is a [`ref struct`][3], which means it can only be allocated on the runtime stack, ensuring that cleanup semantics are *scope* semantics. TODO: is that actually a good idea? If a `JavaPeerableRegistrationScope` is created using `JavaPeerableRegistrationScopeCleanup.RegisterWithManager`, existing behavior is followed. This is useful for nested scopes, should instances need to be registered with `JniRuntime.ValueManager`. If a `JavaPeerableRegistrationScope` is created using `JavaPeerableRegistrationScopeCleanup.Dispose` or `JavaPeerableRegistrationScopeCleanup.Release`, then: 1. `IJavaPeerable` instances created within the scope are "thread-local": they can be *used* by other threads, but `JniRuntime.JniValueManager.PeekPeer()` will only find the value on the creating thread. 2. At the end of a `using` block / when `JavaScope.Dispose()` is called, all collected instances will be `Dispose()`d (with `.Dispose`) or released (with `.Release`), and left to the GC to eventually finalize. For example: using (new JavaPeerableRegistrationScope (JavaPeerableRegistrationScopeCleanup.Dispose)) { var singleton = JavaSingleton.Singleton; // use singleton // If other threads attempt to access `JavaSingleton.Singleton`, // they'll create their own peer wrapper } // `singleton.Dispose()` is called at the end of the `using` block However, if e.g. the singleton instance is already accessed, then it won't be added to the registration scope and won't be disposed: var singleton = JavaSingleton.Singleton; // singleton is registered with the ValueManager // later on the same thread or some other threa… using (new JavaPeerableRegistrationScope (JavaPeerableRegistrationScopeCleanup.Dispose)) { var anotherSingleton = JavaSingleton.Singleton; // use anotherSingleton… } then `anotherSingleton` will *not* disposed, as it already existed. *Only newly created instances* are added to the current scope. By default, only `JavaPeerableRegistrationScopeCleanup.RegisterWithManager` is supported. To support the other cleanup modes, `JniRuntime.JniValueManager.SupportsPeerableRegistrationScopes` must return `true`, which in turn requires that: * `JniRuntime.JniValueManager.AddPeer()` calls `TryAddPeerToRegistrationScope()`, * `JniRuntime.JniValueManager.RemovePeer()` calls `TryRemovePeerFromRegistrationScopes()` * `JniRuntime.JniValueManager.PeekPeer()` calls `TryPeekPeerFromRegistrationScopes()`. See `ManagedValueManager` for an example implementation. Finally, add the following methods to `JniRuntime.JniValueManager` to help further assist peer management: partial class JniRuntime.JniValueManager { public virtual bool CanCollectPeers { get; } public virtual bool CanReleasePeers { get; } public virtual void ReleasePeers (); } TODO: docs? TODO: *nested* scopes, and "bound" vs. "unbound" instance construction around `JniValueManager.GetValue<T>()` or `.CreateValue<T>()`, and *why* they should be treated differently. TODO: Should `CreateValue<T>()` be *removed*? name implies it always "creates" a new value, but implementation will return existing instances, so `GetValue<T>()` alone may be better. One related difference is that` `CreateValue<T>()` uses `PeekBoxedObject()`, while `GetValue<T>()` doesn't. *Should* it? [0]: https://web.archive.org/web/20211106214514/https://bugzilla.xamarin.com/25/25443/bug.html#c63 [1]: https://issuetracker.google.com/issues/37034307 [2]: https://developer.android.com/reference/android/graphics/Typeface#create(java.lang.String,%20int) [3]: https://docs.microsoft.com/dotnet/csharp/language-reference/builtin-types/struct#ref-struct
Fixes: #4 Context: #426 Context: a666a6f Alternate names? * JavaScope * JavaPeerableCleanupPool * JavaPeerCleanup * JavaReferenceCleanup * JniPeerRegistrationScope Issue #426 is an idea to implement a *non*-GC-Bridged `JniRuntime.JniValueManager` type, primarily for use with .NET Core. This was begun in a666a6f. What's missing is the answer to a question: what to do about `JniRuntime.JniValueManager.CollectPeers()`? With a Mono-style GC bridge, `CollectPeers()` is `GC.Collect()`. In a666a6f with .NET Core, `CollectPeers()` calls `IJavaPeerable.Dispose()` on all registered instances, which is "extreme". @jonpryor thought that if there were a *scope-based* way to selectively control which instances were disposed, that might be "better" and more understandable. Plus, this is issue #4! Which brings us to the background for Issue #4, which touches upon [bugzilla 25443][0] and [Google issue 37034307][1]: Java.Interop attempts to provide referential identity for Java objects: if two separate Java methods invocations return the same Java instance, then the bindings of those methods should also return the same instance. This is "fine" on the surface, but has a few related issues: 1. JNI Global References are a limited resource: Android only allows ~52000 JNI Global References to exist, which sounds like a lot, but might not be. 2. Because of (1), it is not uncommon to want to use `using` blocks to invoke `IJavaPeerable.Dispose()` to release JNI Global References. 3. However, because of object identity, you could unexpectedly introduce *cross-thread sharing* of `IJavaPeerable` instances. This might not be at all obvious; for example, in the Android 5 timeframe, [`Typeface.create()`][2] wouldn't necessarily return new Java instances, but may instead return cached instances. Meaning that this: var t1 = new Thread(() => { using var typeface = Typeface.Create("Family", …); // use typeface… }); var t2 = new Thread(() => { using var typeface = Typeface.Create("Family", …); // use typeface… }); t1.Start(); t2.Start(); t1.Join(); t2.Join(); could plausibly result in `ObjectDisposedException`s (or worse), as the threads could be unexpectedly sharing the same bound instance. Which *really* means that you can't reliably call `Dispose()`, unless you *know* you created that instance: using var safe = new Java.Lang.Double(42.0); // I created this, therefore I control all access and can Dispose() it using var unsafe = Java.Lang.Double.ValueOf(42.0); // I have no idea who else may be using this instance! Attempt to address both of these scenarios -- a modicum of .NET Core support, and additional sanity around JNI Global Reference lifetimes -- by introducing a new `JavaPeerableRegistrationScope` type, which introduces a scope-based mechanism to control when `IJavaPeerable` instances are cleaned up: public enum JavaPeerableRegistrationScopeCleanup { RegisterWithManager, Dispose, Release, } public ref struct JavaPeerableRegistrationScope { public JavaPeerableRegistrationScope(JavaPeerableRegistrationScopeCleanup cleanup); public void Dispose(); } `JavaPeerableRegistrationScope` is a [`ref struct`][3], which means it can only be allocated on the runtime stack, ensuring that cleanup semantics are *scope* semantics. TODO: is that actually a good idea? If a `JavaPeerableRegistrationScope` is created using `JavaPeerableRegistrationScopeCleanup.RegisterWithManager`, existing behavior is followed. This is useful for nested scopes, should instances need to be registered with `JniRuntime.ValueManager`. If a `JavaPeerableRegistrationScope` is created using `JavaPeerableRegistrationScopeCleanup.Dispose` or `JavaPeerableRegistrationScopeCleanup.Release`, then: 1. `IJavaPeerable` instances created within the scope are "thread-local": they can be *used* by other threads, but `JniRuntime.JniValueManager.PeekPeer()` will only find the value on the creating thread. 2. At the end of a `using` block / when `JavaPeerableRegistrationScope.Dispose()` is called, all collected instances will be `Dispose()`d (with `.Dispose`) or released (with `.Release`), and left to the GC to eventually finalize. For example: using (new JavaPeerableRegistrationScope (JavaPeerableRegistrationScopeCleanup.Dispose)) { var singleton = JavaSingleton.Singleton; // use singleton // If other threads attempt to access `JavaSingleton.Singleton`, // they'll create their own peer wrapper } // `singleton.Dispose()` is called at the end of the `using` block However, if e.g. the singleton instance is already accessed, then it won't be added to the registration scope and won't be disposed: var singleton = JavaSingleton.Singleton; // singleton is registered with the ValueManager // later on the same thread or some other threa… using (new JavaPeerableRegistrationScope (JavaPeerableRegistrationScopeCleanup.Dispose)) { var anotherSingleton = JavaSingleton.Singleton; // use anotherSingleton… } then `anotherSingleton` will *not* disposed, as it already existed. *Only newly created instances* are added to the current scope. By default, only `JavaPeerableRegistrationScopeCleanup.RegisterWithManager` is supported. To support the other cleanup modes, `JniRuntime.JniValueManager.SupportsPeerableRegistrationScopes` must return `true`, which in turn requires that: * `JniRuntime.JniValueManager.AddPeer()` calls `TryAddPeerToRegistrationScope()`, * `JniRuntime.JniValueManager.RemovePeer()` calls `TryRemovePeerFromRegistrationScopes()` * `JniRuntime.JniValueManager.PeekPeer()` calls `TryPeekPeerFromRegistrationScopes()`. See `ManagedValueManager` for an example implementation. Additionally, add the following methods to `JniRuntime.JniValueManager` to help further assist peer management: partial class JniRuntime.JniValueManager { public virtual bool CanCollectPeers { get; } public virtual bool CanReleasePeers { get; } public virtual void ReleasePeers (); } Finally, many of the new members are marked with the [`[Experimental]` attribute][4], which indicates that an API is experimental and could change. In order to use the new APIs, then the `JI9999` warning must be disabled, e.g. via [`#pragma warning disable JI9999`][5]. This will allow us to break API and semantics, should that be necessary. TODO: docs? TODO: *nested* scopes, and "bound" vs. "unbound" instance construction around `JniValueManager.GetValue<T>()` or `.CreateValue<T>()`, and *why* they should be treated differently. TODO: Should `CreateValue<T>()` be *removed*? name implies it always "creates" a new value, but implementation will return existing instances, so `GetValue<T>()` alone may be better. One related difference is that` `CreateValue<T>()` uses `PeekBoxedObject()`, while `GetValue<T>()` doesn't. *Should* it? [0]: https://web.archive.org/web/20211106214514/https://bugzilla.xamarin.com/25/25443/bug.html#c63 [1]: https://issuetracker.google.com/issues/37034307 [2]: https://developer.android.com/reference/android/graphics/Typeface#create(java.lang.String,%20int) [3]: https://docs.microsoft.com/dotnet/csharp/language-reference/builtin-types/struct#ref-struct [4]: https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.experimentalattribute?view=net-8.0 [5]: https://learn.microsoft.com/dotnet/csharp/language-reference/preprocessor-directives#pragma-warning
Fixes: #4 Context: #426 Context: a666a6f Alternate names? * JavaScope * JavaPeerableCleanupPool * JavaPeerCleanup * JavaReferenceCleanup * JniPeerRegistrationScope Issue #426 is an idea to implement a *non*-GC-Bridged `JniRuntime.JniValueManager` type, primarily for use with .NET Core. This was begun in a666a6f. What's missing is the answer to a question: what to do about `JniRuntime.JniValueManager.CollectPeers()`? With a Mono-style GC bridge, `CollectPeers()` is `GC.Collect()`. In a666a6f with .NET Core, `CollectPeers()` calls `IJavaPeerable.Dispose()` on all registered instances, which is "extreme". @jonpryor thought that if there were a *scope-based* way to selectively control which instances were disposed, that might be "better" and more understandable. Plus, this is issue #4! Which brings us to the background for Issue #4, which touches upon [bugzilla 25443][0] and [Google issue 37034307][1]: Java.Interop attempts to provide referential identity for Java objects: if two separate Java methods invocations return the same Java instance, then the bindings of those methods should also return the same instance. This is "fine" on the surface, but has a few related issues: 1. JNI Global References are a limited resource: Android only allows ~52000 JNI Global References to exist, which sounds like a lot, but might not be. 2. Because of (1), it is not uncommon to want to use `using` blocks to invoke `IJavaPeerable.Dispose()` to release JNI Global References. 3. However, because of object identity, you could unexpectedly introduce *cross-thread sharing* of `IJavaPeerable` instances. This might not be at all obvious; for example, in the Android 5 timeframe, [`Typeface.create()`][2] wouldn't necessarily return new Java instances, but may instead return cached instances. Meaning that this: var t1 = new Thread(() => { using var typeface = Typeface.Create("Family", …); // use typeface… }); var t2 = new Thread(() => { using var typeface = Typeface.Create("Family", …); // use typeface… }); t1.Start(); t2.Start(); t1.Join(); t2.Join(); could plausibly result in `ObjectDisposedException`s (or worse), as the threads could be unexpectedly sharing the same bound instance. Which *really* means that you can't reliably call `Dispose()`, unless you *know* you created that instance: using var safe = new Java.Lang.Double(42.0); // I created this, therefore I control all access and can Dispose() it using var unsafe = Java.Lang.Double.ValueOf(42.0); // I have no idea who else may be using this instance! Attempt to address both of these scenarios -- a modicum of .NET Core support, and additional sanity around JNI Global Reference lifetimes -- by introducing a new `JavaPeerableRegistrationScope` type, which introduces a scope-based mechanism to control when `IJavaPeerable` instances are cleaned up: public enum JavaPeerableRegistrationScopeCleanup { RegisterWithManager, Dispose, Release, } public ref struct JavaPeerableRegistrationScope { public JavaPeerableRegistrationScope(JavaPeerableRegistrationScopeCleanup cleanup); public void Dispose(); } `JavaPeerableRegistrationScope` is a [`ref struct`][3], which means it can only be allocated on the runtime stack, ensuring that cleanup semantics are *scope* semantics. TODO: is that actually a good idea? If a `JavaPeerableRegistrationScope` is created using `JavaPeerableRegistrationScopeCleanup.RegisterWithManager`, existing behavior is followed. This is useful for nested scopes, should instances need to be registered with `JniRuntime.ValueManager`. If a `JavaPeerableRegistrationScope` is created using `JavaPeerableRegistrationScopeCleanup.Dispose` or `JavaPeerableRegistrationScopeCleanup.Release`, then: 1. `IJavaPeerable` instances created within the scope are "thread-local": they can be *used* by other threads, but `JniRuntime.JniValueManager.PeekPeer()` will only find the value on the creating thread. 2. At the end of a `using` block / when `JavaPeerableRegistrationScope.Dispose()` is called, all collected instances will be `Dispose()`d (with `.Dispose`) or released (with `.Release`), and left to the GC to eventually finalize. For example: using (new JavaPeerableRegistrationScope (JavaPeerableRegistrationScopeCleanup.Dispose)) { var singleton = JavaSingleton.Singleton; // use singleton // If other threads attempt to access `JavaSingleton.Singleton`, // they'll create their own peer wrapper } // `singleton.Dispose()` is called at the end of the `using` block However, if e.g. the singleton instance is already accessed, then it won't be added to the registration scope and won't be disposed: var singleton = JavaSingleton.Singleton; // singleton is registered with the ValueManager // later on the same thread or some other threa… using (new JavaPeerableRegistrationScope (JavaPeerableRegistrationScopeCleanup.Dispose)) { var anotherSingleton = JavaSingleton.Singleton; // use anotherSingleton… } then `anotherSingleton` will *not* disposed, as it already existed. *Only newly created instances* are added to the current scope. By default, only `JavaPeerableRegistrationScopeCleanup.RegisterWithManager` is supported. To support the other cleanup modes, `JniRuntime.JniValueManager.SupportsPeerableRegistrationScopes` must return `true`, which in turn requires that: * `JniRuntime.JniValueManager.AddPeer()` calls `TryAddPeerToRegistrationScope()`, * `JniRuntime.JniValueManager.RemovePeer()` calls `TryRemovePeerFromRegistrationScopes()` * `JniRuntime.JniValueManager.PeekPeer()` calls `TryPeekPeerFromRegistrationScopes()`. See `ManagedValueManager` for an example implementation. Additionally, add the following methods to `JniRuntime.JniValueManager` to help further assist peer management: partial class JniRuntime.JniValueManager { public virtual bool CanCollectPeers { get; } public virtual bool CanReleasePeers { get; } public virtual void ReleasePeers (); } Finally, many of the new members are marked with the [`[Experimental]` attribute][4], which indicates that an API is experimental and could change. In order to use the new APIs, then the `JI9999` warning must be disabled, e.g. via [`#pragma warning disable JI9999`][5]. This will allow us to break API and semantics, should that be necessary. TODO: docs? TODO: *nested* scopes, and "bound" vs. "unbound" instance construction around `JniValueManager.GetValue<T>()` or `.CreateValue<T>()`, and *why* they should be treated differently. TODO: Should `CreateValue<T>()` be *removed*? name implies it always "creates" a new value, but implementation will return existing instances, so `GetValue<T>()` alone may be better. One related difference is that` `CreateValue<T>()` uses `PeekBoxedObject()`, while `GetValue<T>()` doesn't. *Should* it? [0]: https://web.archive.org/web/20211106214514/https://bugzilla.xamarin.com/25/25443/bug.html#c63 [1]: https://issuetracker.google.com/issues/37034307 [2]: https://developer.android.com/reference/android/graphics/Typeface#create(java.lang.String,%20int) [3]: https://docs.microsoft.com/dotnet/csharp/language-reference/builtin-types/struct#ref-struct [4]: https://learn.microsoft.com/dotnet/api/system.diagnostics.codeanalysis.experimentalattribute?view=net-8.0 [5]: https://learn.microsoft.com/dotnet/csharp/language-reference/preprocessor-directives#pragma-warning
As a proof-of-concept, can we support "one-way" Java object use, in which construction and use of bound types is supported, but no Java-to-managed object references.
This would mean:
List<Java.Lang.String>
worksIdeally, such a limited construct could be done via a
JniRuntime.JniValueManager
subclass.The text was updated successfully, but these errors were encountered: