RFR: 8344332: (bf) Migrate DirectByteBuffer to use java.lang.ref.Cleaner [v13]

Kim Barrett kbarrett at openjdk.org
Mon Feb 24 06:13:04 UTC 2025


On Tue, 18 Feb 2025 14:33:16 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> DirectByteBuffers are still using old `jdk.internal.ref.Cleaner` implementation. That implementation carries a doubly-linked list, and so makes DBB suffer from the same issue fixed for generic `java.lang.ref.Cleaner` users with [JDK-8343704](https://bugs.openjdk.org/browse/JDK-8343704). See the bug for the reproducer.
>> 
>> We can migrate DBBs to use `java.lang.ref.Cleaner`.
>> 
>> There are two pecularities during this rewrite.
>> 
>> First, the old ad-hoc `Cleaner` implementation used to exit the VM when cleaning action failed. I presume it was to avoid memory leak / accidental reuse of the buffer. I moved the relevant block to `Deallocator` directly. Unfortunately, I cannot easily test it.
>> 
>> Second is quite a bit hairy. Old DBB cleaning code was hooked straight into `Reference` processing loop. This was possible because we could infer that the weak references we are processing were DBB cleaning actions, since old `Cleaner` was the only use of this code. With standard `Cleaner`, we have lost this association, and so we cannot really do this from the reference processing loop. With the patched version, we now rely on normal `Cleaner` thread to do cleanups for us.
>> 
>> Because of this, there is a new outpacing opportunity window where reference processing might have been over, but cleaner thread has not reacted yet. This is why we need another way to check progress that involves checking if cleaner has acted.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 server fastdebug, `java/nio java/io`
>>  - [x] Linux AArch64 server fastdebug, `java/nio java/io`
>>  - [x] Linux x86_64 server fastdebug, `all`
>>  - [x] Linux AArch64 server fastdebug, `all`
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   A bit more comments

> (2) Should Cleaner be responsible for providing the additional functionality?
> [...]
> If there were multiple (potential) clients for this sort of thing then putting
> it in Cleaner may makes sense [...]

And here is a bug and recently opened PR that would benefit from having
Cleaner.waitForCleaning.
https://bugs.openjdk.org/browse/JDK-8204868
java/util/zip/ZipFile/TestCleaner.java still fails with "cleaner failed to clean zipfile."
https://github.com/openjdk/jdk/pull/23742

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

PR Comment: https://git.openjdk.org/jdk/pull/22165#issuecomment-2677514285


More information about the core-libs-dev mailing list