[foreign-abi] RFR: 8253823: Investigate ways to make handoff-like operation more explicit [v2]

[Maurizio Cimadamore]

On Wed, 30 Sep 2020 11:57:36 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revert commented lines in StdLibTest
>
> test/jdk/java/foreign/TestNative.java line 176:
> 
>> 174:         MemoryAddress addr = allocate(12);
>> 175:         MemorySegment mallocSegment = addr.asSegmentRestricted(12)
>> 176:                 .handoff(HandoffTransform.ofShared().addCleanupAction(() -> free(addr)));
> 
> Should this really be using ofShared() ? We don't have a withOwnerThread(null) currently either., and the current
> asSegmentRestricted code ends up making a confined scope as well.

I can change this so that it matches exactly the old code

> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 409:
> 
>> 407:      * hand-over from the current owner thread to the new owner thread, which in turn <i>happens before</i> read
>> accesses 408:      * to the returned segment's contents on the new owner thread.
>> 409:      *
> 
> Maybe it's worth mentioning here and for the other overload that cleanup actions are not run when doing a handoff, and
> they are instead transferred to the new segment. While Cleaners registered to the old segment will have no effect
> anymore, and will have to be re-registered to the new segment.

So, mentioning that cleanup actions are called on `MemorySegment#close` is a good thing.
Regarding Cleaners - your comment makes me wonder - cleaners are the only thing that is NOT propagated, should we
propagate them for consistency; this way what you get out of handoff is a segment that has SAME characteristics as old
segment - except some _additional_ stuff and, possibly, a different owner thread.

> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/NativeScope.java line 37:
> 
>> 35: import java.nio.ByteOrder;
>> 36: import java.util.OptionalLong;
>> 37: import java.util.function.Consumer;
> 
> Seems to be an unused import?

will fix

> src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemorySegment.java line 440:
> 
>> 438:      * @throws IllegalStateException if this segment is not <em>alive</em>, or if access occurs from a thread
>> other than the 439:      * thread owning this segment.
>> 440:      * @throws UnsupportedOperationException if this segment does not support the {@link #HANDOFF} access mode.
> 
> This needs to be added to the other handoff overload as well.

will fix

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

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