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

[Aleksey Shipilev]

On Fri, 15 Nov 2024 20:54:56 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.
> 
> 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`

FTR, wider testing shows two tests that install custom system classloader fail with the following circularity:


TEST: runtime/cds/appcds/SpecifySysLoaderProp.java
TEST: java/security/SignedJar/SignedJarWithCustomClassLoader.java

Caused by: java.lang.IllegalStateException: getSystemClassLoader cannot be called during the system class loader instantiation
        at java.lang.ClassLoader.getSystemClassLoader(java.base at 25-internal/ClassLoader.java:1849)
        at jdk.internal.misc.InnocuousThread.newThread(java.base at 25-internal/InnocuousThread.java:68)
        at jdk.internal.ref.CleanerImpl$InnocuousThreadFactory.newThread(java.base at 25-internal/CleanerImpl.java:208)
        at jdk.internal.ref.CleanerImpl.start(java.base at 25-internal/CleanerImpl.java:110)
        at java.lang.ref.Cleaner.create(java.base at 25-internal/Cleaner.java:174)
        at java.nio.DirectByteBuffer.<clinit>(java.base at 25-internal/DirectByteBuffer.java:95)
        at jdk.internal.perf.Perf.createLong(java.base at 25-internal/Native Method)
        at jdk.internal.perf.PerfCounter.<init>(java.base at 25-internal/PerfCounter.java:63)
        at jdk.internal.perf.PerfCounter.newPerfCounter(java.base at 25-internal/PerfCounter.java:69)
        at jdk.internal.perf.PerfCounter$CoreCounters.<clinit>(java.base at 25-internal/PerfCounter.java:126)
        at jdk.internal.perf.PerfCounter.getZipFileOpenTime(java.base at 25-internal/PerfCounter.java:176)
        at java.util.zip.ZipFile.<init>(java.base at 25-internal/ZipFile.java:203)
        at java.util.zip.ZipFile.<init>(java.base at 25-internal/ZipFile.java:148)
        at java.util.jar.JarFile.<init>(java.base at 25-internal/JarFile.java:333)
        at jdk.internal.loader.URLClassPath$JarLoader.getJarFile(java.base at 25-internal/URLClassPath.java:682)
        at jdk.internal.loader.URLClassPath$JarLoader.ensureOpen(java.base at 25-internal/URLClassPath.java:653)
        at jdk.internal.loader.URLClassPath$JarLoader.<init>(java.base at 25-internal/URLClassPath.java:622)
        at jdk.internal.loader.URLClassPath.getLoader(java.base at 25-internal/URLClassPath.java:465)
        at jdk.internal.loader.URLClassPath.getLoader(java.base at 25-internal/URLClassPath.java:409)
        at jdk.internal.loader.URLClassPath.getResource(java.base at 25-internal/URLClassPath.java:331)
        at jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(java.base at 25-internal/BuiltinClassLoader.java:688)
        at jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(java.base at 25-internal/BuiltinClassLoader.java:620)
        at jdk.internal.loader.BuiltinClassLoader.loadClass(java.base at 25-internal/BuiltinClassLoader.java:578)
        at java.lang.ClassLoader.loadClass(java.base at 25-internal/ClassLoader.java:490)
        at java.lang.Class.forName0(java.base at 25-internal/Native Method)
        at java.lang.Class.forName(java.base at 25-internal/Class.java:543)
        at java.lang.ClassLoader.initSystemClassLoader(java.base at 25-internal/ClassLoader.java:1887)
        at java.lang.System.initPhase3(java.base at 25-internal/System.java:1964)


This can be avoided if we use the lazy holder pattern to delay `Cleaner` initialization until the actual use. Done in new commit.

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

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