Skip to content
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

Open
jonpryor opened this issue May 16, 2019 · 12 comments
Open

Reference Counting, non-bridged backend #426

jonpryor opened this issue May 16, 2019 · 12 comments
Labels
enhancement Proposed change to current functionality java-interop Runtime bridge between .NET and Java

Comments

@jonpryor
Copy link
Member

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:

  • Subclassing is (probably) out
    • though Java-to-managed method invocations should still be possible (static methods?)?
  • Managed code could refer to Java instances, e.g. List<Java.Lang.String> works
  • Java code can't refer to managed code, e.g. if you could subclass, that subclass couldn't contain any fields which reference any reference types. (Which is why subclassing would be out.)

Ideally, such a limited construct could be done via a JniRuntime.JniValueManager subclass.

@jpobst jpobst added enhancement Proposed change to current functionality java-interop Runtime bridge between .NET and Java labels Apr 6, 2020
jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 18, 2021
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
@jonpryor
Copy link
Member Author

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:

  1. The managed peer holds a JNI Global Reference to the Java peer, as always. This keeps the Java peer alive.
  2. The JniRuntime.JniValueManager holds a strong reference to the managed peer. (The JniValueManager is in turn referenced by the JniRuntime instance, which in turn is rooted by a static variable.)

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.)

  • Java code can still refer to Java peers, and as the Java peer is referenced by a JNI Global Reference, it's a root, and thus will always be kept alive by Java.
  • If Java code invokes a Java-Callable-Wrapper constructor for a Managed subclass, that will cause the Managed peer to be constructed, and the JniValueManager will reference it, keeping the Managed peer alive.

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…

@jonpryor
Copy link
Member Author

jonpryor commented Feb 19, 2021

To help "support"/"address" the coordination problem, PR #804 adds two members to JniRuntime.JniValueManager:

partial class JniRuntime {
    partial class JniValueManager {
        public abstract bool PeersRequireRelease {get;}
        public abstract void ReleasePeers ();
    }
}

The PeersRequireRelease property is a way of asking the value manager "is there GC integration?" If PeerRequireRelease is true, there is no GC integration.

The ReleasePeers() method releases all JNI Global References held by Managed peers referenced by the value manager, and stops referencing the managed peers. This allows the respective Java & managed GC's to collect their objects, as there is no longer anything keeping the instances alive.

There is a broader question of semantics: should ReleasePeers() "notify" the peers that they're being released?

There are currently two "lifetime" methods:

IJavaPeerable.Disposed() is invoked e.g. when someone calls IDisposable.Dispose() on a JavaObject or JavaException subclass.

IJavaPeerable.Finalized() is invoked when the GC invokes the finalizer of a JavaObject or JavaException subclass.

See e.g. Java.Interop.JavaObject, in which IDisposable.Dispose() calls JniValueManager.DisposePeer() which calls IJavaPeerable.Disposed() which calls Dispose(disposing:true):

https://github.com/xamarin/java.interop/blob/89a5a2297a7a79f9a0d769a429f80c8b459823d9/src/Java.Interop/Java.Interop/JavaObject.cs#L99-L102

https://github.com/xamarin/java.interop/blob/89a5a2297a7a79f9a0d769a429f80c8b459823d9/src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs#L140-L159

https://github.com/xamarin/java.interop/blob/89a5a2297a7a79f9a0d769a429f80c8b459823d9/src/Java.Interop/Java.Interop/JavaObject.cs#L139-L142

In a CoreCLR environment, IJavaPeerable.Finalized() will eventually be executed, after ReleasePeers() is invoked and the GC (eventually) subsequently collects the instance.

Question: Should ReleasePeers() invoke the Disposed() "event" on the peers? Should there instead be another IJavaPeerable "event" to indicate that ReleasePeers() has been invoked? (If so, it should presumably be called before the GREF is freed, so that it can communicate with the Java peer.)

jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 19, 2021
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
@jonpryor
Copy link
Member Author

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 Example.Singleton:

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 Example.Singleton, it's possible that the single registered instance will be disposed by the above thread A with the using block.

The current "solution" is, in effect, Don't Do That™: don't use using blocks on JavaObject subclasses unless you know you're creating the instance, i.e. you're using a new expression.

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 a is an alias to a to the "single registered instance", not the single registered instance itself.

@jonpryor
Copy link
Member Author

Proposal: Instead of JniEnvironment.BeginGetValueBehaviors(), we tie into JniValueManager, in a "stack-like" manner:

public enum JniValueManagerCollectsRegistrations {
    Never,
    WithSystemGC,
}
partial class JniValueManager {
    public abstract JniValueManagerCollectsRegistrations JniValueManagerCollectsRegistrations {get;}
    public abstract IDisposable CreateRegistrationScope ();
}

The CreateRegistrationScope() "solves" the JniEnvironment.BeginGetValueBehaviors() (#4) scenario, and the we don't want to release everything scenario, by scoping the Release. Calling IDisposable.Dispose() on the value returned will dispose all values created "in" that scope:

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,
}

.Dispose would cause .CreateRegistrationScope(…).Dispose() to call .Dispose() on all registered instances, while .Release would not. ("Solving" the original design conundrum: let someone else deal with it.)

Only one of Global | ThreadLocal can be provided, ditto Dispose | Release. (Separate enum types + parameters? Names?)

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 .Global idea may be a non-starter, and only .ThreadLocal makes sense.

May also want/need to update JniObjectReferenceOptions to allow explicitly registering globally:

public partial enum JniObjectReferenceOptions {
    CopyAndRegisterGlobal = Copy + (1 << 3),
}

Current JniObjectReferenceOptions.CopyAndDoNotRegister support also needs review and possible re-thinking.

@jonpryor
Copy link
Member Author

Design thoughts…

Instead of a bool PeersRequireRelease {get;} property, perhaps it should be an enum of some form?

ReleasePeers() doesn't need to be the abstraction; it could be implemented in terms of GetSurfacedPeers(). It could be an extension method. (Not that it should, but it could…)

Additionally, ReleasePeers() is too much.

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.

ReleasePeers() does not work with such a construct. It nukes everything.

I think we need a better abstraction than what is proposed here. Probably something "stack-like".

@jonpryor
Copy link
Member Author

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.

jonpryor added a commit to jonpryor/java.interop that referenced this issue Feb 19, 2021
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
@jonpryor
Copy link
Member Author

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 JniValueManager as the "main" entry point is not good: it's too many .s, it's too hard to find. Instead, we should consider a new type as the primary entry point:

[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" (?) @autoreleasepool scope semantics:

using (var scope = new JniPeerRegistrationScope()) {
    var list = new Java.Util.ArrayList ();
}
// at scope.Dispose(), `list` and everything it contains is automatically .Dispose()d.

JniPeerRegistrationScope internals would be a thread-local construct. Values created on other threads would not use the same registration scope; they would have their own, or the "root global" scope.

This in turn begs the question, how do we ensure it's thread-local? The answer here may be to be instead use a ref struct, which would prevent storing the instance into the GC heap.

Pity ref struct can't use interfaces…

However, using doesn't require IDisposable, so this works with the previous example, except that all constructor parameters cannot be optional. (They "can" have default values, but for some reason on mono 6.12, the constructor body isn't executed unless the construction-site provides parameters. ¯_(ツ)_/¯)

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 System.Reflection.BindingFlags. Perhaps a separate struct is warranted?

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 ();
}

@jonpryor
Copy link
Member Author

…and more thinking, the current DoNotRegister idea isn't really viable, as currently constructed, because there may be an entire object graph implicitly created, but -- in the idea of the singleton, or Typeface.create() -- you only have access to the one reference, not the whole object graph.

Instead, to support Issue #4, we'd want a CreateNew… semantic, wherein "object identity" is not preserved, and all new objects are "registered somewhere", so that at scope dispose all of the created instances are reachable and can be Disposed.

I'm increasingly thinking that JniPeerRegistrationScope need not have "public" changes to JniValueManager API, but instead to semantics.

A thread-local "scope stack" should be part of JniEnvironmentInfo, so that we only have one TLS slot to worry about/in use:

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;
    }
}

@jonpryor
Copy link
Member Author

"Related" random thought: there is a System.Threading.ThreadPool.EnableDispatchAutoreleasePool feature switch: https://github.com/dotnet/runtime/blob/d0616f837f4ad1ceb7e36a341934c929fd790089/docs/workflow/trimming/feature-switches.md

When set to true, creates an NSAutoreleasePool around each thread pool work item on applicable platforms.

If we had a JniPeerRegistrationScope/JniPeerRegistrationPool type, would it be useful to support this switch with similar "semantics"?

@jonpryor
Copy link
Member Author

Back on the JniValueManager side of things, instead of a new enum, go for an easier semantic. (Still a breaking change vs. current API, but at least it's explicable.)

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 CollectPeers() involves/requires GC integration. "Collecting" peers without GC integration means that we can't ("reasonably efficiently) "mirror" the managed object graph into Java land, preventing the Java-side GC from being necessarily useful. (We could inefficiently do so by using Reflection to traverse the graph of all referenced Peers, trying to find all IJavaPeerable values, and reproduce that graph, but…ouch, that would be terribly inefficient.)

ReleasePeers() is likewise efficient: stop referencing all IJavaPeerable values. Do not .Dispose().

Documentation-wise -- once we have docs -- we should also mention how JniValueManager ties in to identity semantics, especially since we want to make that more "flexible" vis-a-vis the "auto release pool" idea.

jonpryor added a commit to jonpryor/java.interop that referenced this issue May 5, 2021
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
jonpryor added a commit to jonpryor/java.interop that referenced this issue Jun 17, 2021
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
@jonpryor
Copy link
Member Author

A possible alternate/additional idea suggested by @wasabi#2184:

Ever thought about using DependentHandles for the GC stuff? Not going to be performant. But might work.

What's a "dependent handle"? ConditionalWeakTable<TKey, TValue> uses them internally: https://yizhang82.dev/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 ManagedHandle, which is kept alive via static variables:

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 ConditionalWeakTable<ManagedHandle, IJavaPeerable>.

ManagedHandle.Alloc() invocation is straightforward enough. But what calls ManagedHandle.Free()? For that, we use Java finalizers / Cleaner / etc.:

// Java
/* partial */ class ManagedHandle {
    public ManagedHandle();
    @Override protected void finalize() { Runtime.freeManagedHandle(id); }
}

Because of ManagedHandle.handles, all ManagedHandle instances will be kept alive by the managed GC. However, when the "Java peer" of ManagedHandle is finalized/cleaned up/whatever, then the ManagedHandle instance will be removed from ManagedHandle.handles.

The setup:

class MyObject : Java.Lang.Object {}

var o = new MyObject();

A problem here is that in "steady state" nothing can be collected: managed o has a GREF keeping the Java instance alive, which would keep the ManagedHandle alive. There's nothing to "break" the circular reference.

Here, we could have JniValueManager.CollectPeers() toggle all handles and trigger a Java-side GC, which would (eventually) allow the Java-side ManagedHandle to be collected, which would remove the ManagedHandle.handles entry, which would allow it to be collected. However, the timing here feels "ugly".

More pondering required.

@jonpryor
Copy link
Member Author

jonpryor added a commit that referenced this issue Aug 2, 2024
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
jonpryor added a commit that referenced this issue Aug 6, 2024
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
jonpryor added a commit that referenced this issue Aug 7, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Proposed change to current functionality java-interop Runtime bridge between .NET and Java
Projects
None yet
Development

No branches or pull requests

2 participants