On Fri, 15 Nov 2024 20:54:56 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 are in DBB cleaning action, 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. > > Tests show that DirectBufferAlloc tests are still surviving this, possibly > due to exponential sleep-backoff already built in. See the reclamation path > in `Bits.unreserveMemory`: > https://github.com/openjdk/jdk/blob/c207cc7e705d3f449f2387324d86cfb31ce40c44/src/java.base/share/classes/java/nio/Bits.java#L106-L186 > > Additional testing: > - [x] Linux x86_64 server fastdebug, `java/nio java/io` > - [x] Linux AArch64 server fastdebug, `java/nio java/io` > - [ ] Linux x86_64 server fastdebug, `all` > - [ ] Linux AArch64 server fastdebug, `all` src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template line 88: > 86: // Old jdk.internal.ref.Cleaner behavior: when it fails, > VM exits. > 87: if (System.err != null) { > 88: new Error("Cleaner terminated abnormally", > x).printStackTrace(); @shipilev If this line throws, should the exit(1) be skipped? 🤔 ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22165#discussion_r1922401820
