[foreign-memaccess] [Rev 01] RFR: Preserve memory scope for buffer segments

[Paul Sandoz]

On Thu, 16 Apr 2020 16:05:13 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Not 100% sure about this, but wanted to share this PR for evaluation. Basically, I've realized that we are also not
>> preserving the memory scope of the original segment when we do a segment -> buffer -> segment transition.
>> This means that, e.g. code like this:
>> 
>> MemorySegment segment = MemorySegment.allocateNative(16);
>> segment = MemorySegment.ofByteBuffer(segment.asByteBuffer());
>> segment.close(); // no cleanup of native memory
>> 
>> will not really 'close' the original segment. Similarly, if the original segment is being worked upon by some other
>> thread (through a spliterator) it is possible for the morphed segment to lose this info, and to allow a close - if the
>> scope is created afresh (as is now).  This patch fixes this, but I'm torn as to whether it makes things better;
>> preserving access modes and thread ownership is mostly about not allowing clients to use the byte buffer escape hatch
>> as a way to defy the segment restrictions. I don't think preserving scope falls into the same bucket - e.g. this is a
>> separate question as to whether a segment S2 obtained through S1 -> BB -> S2 is just a *view* of S1 (with same temporal
>> bounds), or is just an independent segment, with new temporal bounds. I think both answers are legitimate, and it's
>> mostly a question of making up our mind.  My take is that inferring scope like this can be seen a bit too magic - and
>> you are never sure of what happens when you do a MemorySegment::close, so perhaps what we have w/o this patch is
>> preferrable. What do you think?
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added test (contributed by Jorn Vernee)

test/jdk/java/foreign/TestSharedAccess.java line 142:

> 141:     @Test(expectedExceptions=IllegalStateException.class)
> 142:     public void testBadCloseWithPendingAcquireBuffer() throws InterruptedException {
> 143:         MemorySegment segment = MemorySegment.allocateNative(16);

I think this test can be improved to avoid the 5s execution time, plus a dangling thread hanging around for 500s.  With
some latches we can ensure the main thread calls `segment.close()` while another other thread is within the
`spliterator.forEachRemaining` (and further ensure the runnable task completes before the test completes).  I'll send a
pull request.

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

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