RFR: 8343704: Bad GC parallelism with processing Cleaner queues [v10]

[Aleksey Shipilev]

On Fri, 15 Nov 2024 21:31:54 GMT, Brent Christian <bchristi at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   prev is not needed
>
> src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 310:
> 
>> 308:             }
>> 309:             assert phc.node.arr[phc.index] == phc;
>> 310:             assert head.size > 0;
> 
> I now realize that these assertions in `remove()` won't really "work" because _"exceptions thrown by the cleaning action are ignored"_. :(
> When the cleaner thread removes the phc (`ref.clean()` in `run()` on L142 -> `list.remove()` on PhantomCleanable L94 ), any `AssertionError`s will get lost in `run()`'s `catch` statement, L144.
> 
> (There is a related RFE [JDK-8305979 : UncaughtExceptionHandler for Cleaner](https://bugs.openjdk.org/browse/JDK-8305979))

Ah, true. Yes, this is inconvenient, but I think these asserts are opportunistic to catch internal errors, so they are of interest of JDK testing and catching JDK bugs, not the issues in the user code in Cleaners. I see the asserts would fire from the direct invocation of `clean()`, as deliberately breaking the asserts in `remove()` gets me this failure immediately:


Running test 'jtreg:test/jdk/java/lang/ref'
Directory "/Users/shipilev/Work/shipilev-jdk/build/macosx-aarch64-server-release/test-results/jtreg_test_jdk_java_lang_ref" not found: creating
Error: A JNI error has occurred, please check your installation and try again
Exception in thread "main" java.lang.AssertionError
	at java.base/jdk.internal.ref.CleanerImpl$CleanableList.remove(CleanerImpl.java:310)
	at java.base/jdk.internal.ref.PhantomCleanable.clean(PhantomCleanable.java:94)
	at java.base/java.util.zip.ZipFile$ZipFileInflaterInputStream.close(ZipFile.java:416)
	at java.base/java.util.jar.JarFile.getBytes(JarFile.java:803)
	at java.base/java.util.jar.JarFile.checkForSpecialAttributes(JarFile.java:1006)
	at java.base/java.util.jar.JarFile.hasClassPathAttribute(JarFile.java:954)
	at java.base/java.util.jar.JavaUtilJarAccessImpl.jarFileHasClassPathAttribute(JavaUtilJarAccessImpl.java:34)
	at java.base/jdk.internal.loader.URLClassPath$JarLoader.getClassPath(URLClassPath.java:922)
	at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:447)
	at java.base/jdk.internal.loader.URLClassPath.getResource(URLClassPath.java:314)
	at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:757)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:681)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:639)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:497)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:577)
	at java.base/java.lang.Class.forName(Class.java:556)
	at java.base/sun.launcher.LauncherHelper.loadMainClass(LauncherHelper.java:854)
	at java.base/sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:739)


So I don't regard this as blocking.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22043#discussion_r1846180271