[crac] RFR: Backout new API to sync with Reference Handler [v2]

[Anton Kozlov]

On Thu, 20 Apr 2023 12:27:21 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

>> This reverts commit 9cf1995693eead85d3807fb4c83ab38c14e27042 and makes #22 obsolete. 
>> 
>> The API introduced in 9cf1995693eead85d3807fb4c83ab38c14e27042 (waitForWaiters) and changed in #22 waits for the state when all discovered references are processed. So WaitForWaiters is used to implement predictable Reference Handling, ensuring that clean-up actions have fired for an object after it becomes unreachable.
>> 
>> I think that API was a mistake and should be reverted.
>> 
>> In general, the problem of predictable Reference Handling is independent of CRaC. So I thought about extracting that out of CRaC and found a few issues with the approach. A user needs to know what RefQueue gets References after an object becomes unreachable, to call waitForWaiters on that queue. The queue is not necessarily evident, so a deep understanding of refs and queues in an application is required to select the proper queue to wait on, and to build the right order of them to wait on. Also, it's required somehow to know the number of threads servicing a queue. And there are situations when waitForWaiters may report that all refs are processed, but some of them are not -- consider a thread that is polling a queue and gets refs to be processed but then buffers them in another queue for later, in this example waitForWaiters does not provide the guarantee that corresponding clean-up actions were performed.
>> 
>> The common and more straightforward way to have predictable clean-up is to call an explicit method like close()/release()/cleanup() that performs object-specific clean-up actions predictably.
>
> Anton Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Merge remote-tracking branch 'jdk/crac/crac' into revert-new-ref-handler-api
>  - Backout new API to sync with Reference Handler
>    
>    This reverts commit 9cf1995693eead85d3807fb4c83ab38c14e27042.

jdk/crac/JarFileFactoryCacheTest/JarFileFactoryCacheTest.java failed, investigating.

----------stdout:(7/762)----------
JVM: FD fd=0 type=fifo: details1="pipe:[35938]" OK: inherited from process env
JVM: FD fd=1 type=fifo: details1="pipe:[35939]" OK: inherited from process env
JVM: FD fd=2 type=fifo: details1="pipe:[35940]" OK: inherited from process env
JVM: FD fd=3 type=regular: details1="/home/runner/jdk-linux-x64-debug/jdk-17-internal+0_linux-x64_bin-debug/jdk-17/fastdebug/lib/modules" OK: inherited from process env
JVM: FD fd=4 type=character: details1="/dev/random" OK: always available, random or urandom
JVM: FD fd=5 type=character: details1="/dev/urandom" OK: always available, random or urandom
JVM: FD fd=6 type=regular: details1="/home/runner/work/crac/crac/build/run-test-prebuilt/test-support/jtreg_test_jdk_jdk_crac/scratch/test.jar" BAD: opened by application
----------stderr:(11/718)----------
Exception in thread "main" jdk.crac.CheckpointException
        at java.base/jdk.crac.Core.checkpointRestore1(Core.java:141)
        at java.base/jdk.crac.Core.checkpointRestore(Core.java:246)
        at java.base/jdk.crac.Core.checkpointRestore(Core.java:231)
        at JarFileFactoryCacheTest.exec(JarFileFactoryCacheTest.java:75)
        at jdk.test.lib.crac.CracTest.run(CracTest.java:157)
        at jdk.test.lib.crac.CracTest.main(CracTest.java:89)
        Suppressed: jdk.crac.impl.CheckpointOpenFileException: /home/runner/work/crac/crac/build/run-test-prebuilt/test-support/jtreg_test_jdk_jdk_crac/scratch/test.jar
                at java.base/jdk.crac.Core.translateJVMExceptions(Core.java:87)
                at java.base/jdk.crac.Core.checkpointRestore1(Core.java:145)
                ... 5 more

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

PR Comment: https://git.openjdk.org/crac/pull/34#issuecomment-1517594606