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

[Peter Levart]

On 4/29/20 11:16 PM, Maurizio Cimadamore wrote:
>
> 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.

Think differently: what if the client succeeded in closing the segment, 
just because it did it in a time window when no thread in the thread 
pool held an open scope (this is entirely possible with parallel stream 
for example since threads periodically acquire and close scopes). This 
would have the same effect on threads in the thread pool - they would 
not be able to continue their work... What I'm trying to say is that 
this is just a mechanism to make things safe, not to coordinate work. If 
program wants to avoid trouble, it must carefully coordinate work of 
threads.

>
> 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)

Well, you might not agree, but I don't think of this as a problem. You 
are trying to define some "correct" behavior to a program that is 
incorrectly written anyway. I'm just saying that if the program is 
correctly written, then there is no exceptions. And not defining the 
behavior for incorrect programs is not that uncommon. Just think of Java 
memory model. How much of a program behavior is guaranteed when program 
has data races?

Regards, Peter

>
> Maurizio
>
>>
>> Maurizio
>>