RFR: 8316337: (bf) Concurrency issue in DirectByteBuffer.Deallocator [v3]

[Chen Liang]

On Mon, 18 Sep 2023 14:54:45 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template line 96:
>> 
>>> 94:                 return;
>>> 95:             }
>>> 96:             deallocated = true;
>> 
>> Shouldn't this be an atomic compareAndSet instead? Thread A can read deallocated and pause before the field write, then thread B read deallocated, continue to set the field. There will be a double free when thread A resumes.
>
>> Shouldn't this be an atomic compareAndSet instead? Thread A can read deallocated and pause before the field write, then thread B read deallocated, continue to set the field. There will be a double free when thread A resumes.
> 
> Deallocator is the cleaner action so it should only ever be run once by the cleaner thread. There is a back door via Unsafe.invokeCleaner for freeing a buffer potentially in use but the cleaner action should still only be invoked once.  The "Paranoia" comment and guarding against being called again may date back to the older cleaner code, requires a bit of digging to see if (and the flag) can be removed.

If only one cleaner thread runs, can we just safely assume it is run only once and simply remove the `deallocated` check, which only defends against some, but not all, of those erroneous situations?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15784#discussion_r1328912359