[foreign-memaccess] RFR: Fix concurrent MemoryScope.close() or MemoryScope.dup() race

[Peter Levart]

On Wed, 13 May 2020 16:26:38 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> On 5/12/20 3:55 PM, Maurizio Cimadamore wrote:
>>>
>>> *@mcimadamore* commented on this pull request.
>>>
>>> I've been playing around a bit and discussed this offline with
>>> @JornVernee <https://github.com/JornVernee> too. A realization I had
>>> some time ago is that what we're trying to do here is a read/write
>>> lock, where multiple acquire can occur at the same time, but where
>>> close of the root should be exclusive. I think all the machinery
>>> around CLOSING is essentially due to the code trying to mimic that
>>> pattern.
>>>
>>> I've put together this code:
>>> https://gist.github.com/mcimadamore/54fecd888bcdc8fcf8aefaa51d4cbcbd
>>>
>>> Which, I think, simplifies over the status quo, and also adds back the
>>> atomicity seeked in the proposed patch. Also, since now there some
>>> guarantee that no acquire can take place while a close is also taking
>>> place (and pending acquires will be invalidated - using the optimistic
>>> read logic), then we can just use a single long adder instead of two.
>>>
>>> The state is also simplified, since we just need a boolean flag.
>>>
>>> This seems to provide same numbers as what we have now, but IMHO the
>>> code is bit easier to follow, as there are less moving parts. What do
>>> you think?
>>>
>> 
>> I think this is correct implementation. And perhaps easier to follow
>> too. Nice use of StampedLock which hides the complexity of read-write
>> locking under the easy to use API. And the scalable counter state is
>> reduced by half without reducing scalability too, which is also an
>> improvement - a particular thread typically stabilizes to always update
>> the same cell in the LongAdder so there should be little to no
>> contention even with one counter.
>> 
>> I think this should replace current version. I'm all for it. Good work.
>> 
>> 
>> Regards, Peter
>> 
>> 
>>> —
>>> You are receiving this because you authored the thread.
>>> Reply to this email directly, view it on GitHub
>>> <https://github.com/openjdk/panama-foreign/pull/160#pullrequestreview-410066310>,
>>> or unsubscribe
>>> <https://github.com/notifications/unsubscribe-auth/AAMBYGSBAIYTAMRMR6J5XSTRRFIL7ANCNFSM4M6JYTMQ>.
>>>
>
>> I think this should replace current version. I'm all for it. Good work.
> 
> Cool! Then I suggest you maybe close this PR and I will submit another with the new impl, if that's ok?

Closing this PR in favor of Maurizio's implementation.

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

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