On Tue, 18 Feb 2025 14:33:16 GMT, Aleksey Shipilev <sh...@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
