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

[Marcono1234]

On Tue, 19 Sep 2023 06:33:08 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR proposes to make fields of implementations of the interface `DirectByteBuffer.Deallocator` `final` ensuring safe publication and immutability.
>
> Per Minborg has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Make unmappers truly immutable
>  - Revert to anonymous UmapperProxy class

src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 1200:

> 1198:                 return;
> 1199:             nd.unmap(address, size);
> 1200:             address = 0;

To be safe, would it make sense to keep this code?

For `DirectByteBuffer$Deallocator` you can easily verify that it is safe because the class is only used to create a `Cleaner` (and `Deallocator` is private).

However, here for `Unmapper` `unmap()` is a public method which can be called from other classes through `UnmapperProxy` (currently called by `FileChannelLinesSpliterator#unmap`), and there is also `Unmapper#run` which calls `unmap`. Possibly the current code is still safe and no double-unmap is possible, but verifying this, and ensuring that this remains the case in the future seems difficult. So this additional safeguard here seems useful (at least in my opinion).

(Sorry for this super late comment)

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

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