[foreign-jextract] RFR: Minor change for possible race in shared resource list

[Maurizio Cimadamore]

On Sat, 24 Apr 2021 01:40:40 GMT, Radoslaw Smogura <github.com+7535718+rsmogura at openjdk.org> wrote:

> (1) One or more threads can see "prev" as CLOSED_LIST. Even those CAS will be performed
> and list head will be set to new cleanup. Than exception is going to be thrown.
> 
> (2) Than cleanup will continue (maybe skipping just added cleanup).
> 
> If in addition, other thread will join in parallel to (1) with cleanup2, than
> queue could look like this HEAD = cleanup2 -> list end marker.

To be clear, compareAndExchange returns the value _previously_ set on memory. So, if the actual value happens to be CLOSED_LIST, compareAndExchange would return that. So here:


ResourceCleanup newSegment = (ResourceCleanup) FST.compareAndExchangeRelease(this, prev, cleanup);
                if (newSegment == ResourceCleanup.CLOSED_LIST) {
                    // too late
                    throw new IllegalStateException("Already closed");  
                }


we will execute the code in the if branch and throw - but, sadly, there is a chance that we would also already have replaced the first element of the list (CLOSED_LIST) with something else.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/SharedScope.java line 142:

> 140:                     throw new IllegalStateException("Already closed");
> 141:                 }
> 142:                 cleanup.next = prev;

Why not moving this assigment inside the `if` block (e.g. when `add` succeeded) and then remove the `cleanup.next = null`  (since the assignment would never had been executed in that case) ?

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

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