RFR 8243491: Implementation of Foreign-Memory Access API (Second Incubator)

[Maurizio Cimadamore]

On 29/04/2020 21:40, Maurizio Cimadamore wrote:
>
> On 29/04/2020 08:10, Peter Levart wrote:
>> Right, as you saw in a private Email, I did exactly that in a revised 
>> version (posted below). The spurious ISE may happen but only when 
>> close is called prematurely relative to all child scope close(s) that 
>> were or are still active. So we could say the other way: if close was 
>> not called prematurely, the ISE on acquire would not be spurious - so 
>> ordering of close relative to later acquire was wrong anyway and the 
>> exception is an "indication" of that wrong ordering
>>
>> Unless we want to use close() to "probe" the scope whether it is 
>> still active or not, I don't think this should present a problem.
>
> One quick comment: unless I'm missing something, this is starting to 
> make a pretty strong assumption on how acquire()/close() are going to 
> be used. Yes, if acquire() is not exposed to developers (as in the 
> current API) and only hidden behind a spliterator, we might get away 
> with this; but should we at some point re-introduce some kind of 
> explicit acquire mechanism in the API, then such an assumption would 
> not be a fine one to make.

Actually, the more I think of it, the less I'm convinced that, even in 
the restricted case of acquire() that the API has now, the proposed 
implementation is correct.

A client has a segment, and it creates a spliterator for it. Then it 
gives the spliterator to some thread pool which will happily work away 
with the segment.

But, the client code prematurely closes the segment - this operation 
should fail with exception, as per javadoc, if there are other actors 
working on the segment. Your implementation does that, but that failure 
leaves a sneaky side-effect - in that the threads in the thread-pool 
might not be able to continue their work on the segment.

This action-at-a-distance between a failed close and a pending acquire 
is not documented anywhere, and I also find it very counter-intuitive.

So, while I agree there might be ways to have a more scalable scope 
implementation, I think this is a problem that needs to be addressed.

If we were willing to change the spec a bit, I would then be more 
inclined to say that when you call MemorySegment::close you always close 
- period; under no circumstances is an exception thrown. If there are 
pending acquires on the segment, we keep spinning until there aren't, 
and then we close for good.

I think that would be a more stable semantics, rather than one where it 
seems like the operation failed, but in reality, it succeeded in part 
;-) (at least for a period of time)

Maurizio

>
> Maurizio
>