[foreign-memaccess+abi] RFR: 8265974: ResourceScope code does not handle close vs. add races well

[Paul Sandoz]

On Mon, 26 Apr 2021 13:15:40 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This PR is motivated by the discussion in:
> 
> https://git.openjdk.java.net/panama-foreign/pull/519
> 
> There seems to be an issue (at least conceptually) in that the code which adds a new cleanup can, in principle, replace CLOSED_LIST with a new cleanup action, which then breaks the list invariants.
> 
> While some fixes are possible, I believe that this part of the code is too complex and there's really no reason as to why we would want to allow CLOSE and ADD to happen in parallel with as few synchronization as possible.
> 
> Since we already have a way to prevent a scope from being closed, it only seems fair that `addCloseAction` should defend against scope closure using acquire. Under this new, simplified world, in the shared case we only have to worry about add vs. add races, as add vs. close is no longer possible.
> 
> I've re-ran the benchmark `ResourceScopeClose` and noticed no performance impact of this code simplification. So I think it would be better to make the code "dumber" but more obviously correct, rather than chasing dubious micro optimizations whose correctess we're not 100% sure about.

Looks like a good simplification. At first i thought the `acquire` in `addInternal` need only apply to shared scopes, but the cost for a confined scope is likely minimal. Could specialize later if need be.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/ConfinedScope.java line 90:

> 88:      * A confined resource list; no races are possible here.
> 89:      */
> 90:     static class ConfinedResourceList extends ResourceList {

Remove of `final` needed?

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

Marked as reviewed by psandoz (Committer).

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