[foreign-abi] RFR: 8247439: NativeAllocationScope should have a way to register existing segments onto it

[Maurizio Cimadamore]

On Fri, 12 Jun 2020 03:15:15 GMT, Athijegannathan Sundararajan <sundar at openjdk.org> wrote:

>> In extracted code, there are two cases where segments can be created outside of an allocation scope:
>> 
>> * structs passed returned by value
>> * callback segments
>> 
>> In these cases it is necessary for clients to manually handle the life-cycle of these segments with a separate (nested)
>> try with resource, which makes the code harder to follow.
>> Jextract is currently bypassing this limitation by providing a "register" mechanism, but the provided mechanism is not
>> very safe, since it is up to the user not to prematurely close the registered segment.
>> This patch add a safer routine (called `NativeAllocationScope::claim`) which can be used to attach a segment onto an
>> existing scope. This returns a _new_ segment with a brand _new_ memory scope (and the old segment is killed). The new
>> segment only features mininal access modes (READ and WRITE). As such, it cannot be closed directly - it can only be
>> closed when the allocation scope is.  Furthermore, there are some important restrictions on which segments can be
>> attached to a scope:
>> * the thread owner of the segment must match that of the allocation scope (yes, now allocation scopes are confined too)
>> * the segment to be attached must feature the CLOSE access mode
>> 
>> The latter restriction is important for a number of reasons:
>> 
>> 1. it prevents an attached segment to be re-attached somewhere else (since attached segment cannot feature CLOSE mode
>> by definition) 2. it prevents acquired views to be attached - the lifecycle of acquired views is closely tied to that
>> of the parent segment, and the spliterator they come from - so it doesn't seem a good idea to attach them somewhere
>> else 3. it prevents, more generally, NativeAllocationScope::close to throw because some attached segment cannot be
>> closed  Under the hood, NativeAllocationScope::claim uses the same logic as MemorySegment::withOwnerThread, so we know
>> that logic is safe.
>> I'm open to suggestion for the method name (e.g. maybe `claimOwnership` could be more descriptive?); I'm also open to
>> consider renaming `NativeAllocationScope` to just `NativeScope`.
>
> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/NativeAllocationScope.java line 52:
> 
>> 51: public abstract class NativeAllocationScope implements AutoCloseable {
>> 52:
>> 53:     /**
> 
> Yes, NativeScope is better name - apart from being shorter name, it now does more than just allocation (manages a life
> time of a list of MemorySegments)

Ok, I'll apply the renamings in a follow up patch.

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

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