[foreign-memaccess+abi] RFR: 8263018: Improve API for lifecycle of native resources

[Maurizio Cimadamore]

On Thu, 4 Mar 2021 13:03:42 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This patch brings the API improvements described in [1]. Note that this is a significant reshuffle of the API, and might have non-trivial source compatibility impact; the main moves are listed below:
> 
> * MemorySegment is no longer AutoCloseable (ResourceScope is, and a MemorySegment *has a* ResourceScope)
> 
> * Resources created without an explicit scope (e.g. `MemorySegment::allocateNative(long)`) get a *default* scope, which is non-closeable and GC managed. In other words, users who do not bother with resource scopes, will get same functionalities (lifecycle-wise) that they get with the ByteBuffer API
> 
> * Support for custom allocators is added via the `SegmentAllocator` interface; now all API points which need to perform some allocation will accept an explicit `SegmentAllocator` parameter. Where SegmentAllocator-less overloads are provided, the assumed semantics is that of `MemorySegment::allocateNative(long, long)` which means the returned segments will be associated with the *default scope* and will **not** be closeable.
> 
> * The method handles generated by the linker will accept an additional leading parameter of type `SegmentAllocator` whenever the native function returns a struct by value. There is an overload which allows to statically bind an allocator at MH creation time.
> 
> * The NativeScope abstraction has been removed. This follows the observation that with `SegmentAllocator::arenaBounded/Unbounded` clients can already get pretty close to the functionalities provided by a NativeScope (that said, at least initially, it is likely that jextract will emit a NativeScope abstraction in the generated code, to minimize compatibility impact).
> 
> * As discussed in [1], the new API has a way to prevent a resource scope from being closed when operating on a resource associated with that scope; this is `ResourceScope::acquire` which returns a so called *resource scope handle* (an AutoCloseable).
> 
> * Access modes are gone. We only kept read-only views (as they are needed to support mapped memory). Non-closeable segments can be obtained by using the acquire API.
> 
> * Several methods in MemorySegment are also gone; e.g. all methods related to ownership changes (`withOwnerThread`, `share`) as well as some of the predicate methdos (e.g. `ownerThread`, which is now available through the segment's scope). The overall philosophy is that a scope is assigned to a segment on creation; the scope contains details about how the segment is to be accessed, and these details cannot be altered after the fact.
> 
> * `MemoryAddress::asSegmentRestricted` now takes an optional ResourceScope parameter - the scope to be associated with the returned (unsafe) segment. There is also an overload that doesn't take a ResourceScope, in which case the *global scope* (singleton, non-closeable scope) will be used. A similar argument applies to `VaList.ofAddressRestricted`.
> 
> * The linker will now accept a scope for the returned upcall stub segment - if no scope is provided, a default one (GC-managed, non-closeable) will be created instead. Same for `VaList::make`.
> 
> Overall, I think this patch makes the API cleaner by clarifying the role between lifcycles (e.g. resource scopes) and resources (segments, va lists, etc.) , w/o making clients much more verbose. We also did some internal validation (thanks Chris) to make sure that async byte buffer operation could be adjusted using the *acquire* mechanism. There are some minor outstanding issues which will need to be resolved (as part of this PR, or as a followup) - listed below:
> 
> * should the default scope accept custom cleanup actions? Since this scope is created internally using our cleaner factory, I think there's a possibility that user-defined cleanup action might stall the cleaner forever.
> 
> * should ResourceScope have a predicate to see if it's a default scope? How should it be called? In an earlier iteration I had `isCloseable` which I don't think does a good job (as a scope can also not be closeable for different reasons).
> 
> * Are we ok with the ResourceScope::acquire/ResourceScope.Handle names? Also note that the latter is a simple AutoCloseable, but we need a subclass because the main AutoCloseable::close throws Throwable. Which is unfortunate.
> 
> * What about NativeScope? Are we ok with *not* having it? Note that, without NativeScope, code like:
> 
> try (NativeScope scope = NativeScope.ofUnbounded()) {
>    ...
> }
> 
> becomes:
> 
> try (ResourceScope scope = ResourceScope.ofConfined()) {
>    SegmentAllocator allocator = SegmentAllocator.arenaUnbounded(scope);
>    ...
> }
> 
> E.g. one extra line in the user code. I believe this is not a huge deal, in the sense that in all our example (most of which are jextract based), the body of the try with resources if pretty biggie in comparison. That said, I'm open to reintroduce NativeScope - although probably in a different form - e.g. by having a sub-interface of SegmentAllocator called BoundedAllocator, which is essentially an allocator that *has a* scope. But we can also add this later depending on our experience with using the API.
> 
> That's all for now. Feedback welcome!
> 
> [1] - https://inside.java/2021/01/25/memory-access-pulling-all-the-threads/

Javadoc available here: http://cr.openjdk.java.net/~mcimadamore/panama/resourceScope-javadoc_v3/javadoc/jdk/incubator/foreign/package-summary.html

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/466