[crac] RFR: Remove code trigger register during checkpointRestore

[Anton Kozlov]

On Thu, 25 May 2023 07:33:16 GMT, joeylee <duke at openjdk.org> wrote:

> Register higher priority context during checkpoint could lead to dead lock, this patch removes the code that triggers register during checkpoint call. 
> 
> Thread dump for `CracOptionTest`  without this patch
> 
> 2023-05-23 18:10:39
> Full thread dump OpenJDK 64-Bit Server VM (17-internal+0-adhoc.ubuntu.jdk mixed mode, sharing):
> 
> Threads class SMR info:
> _java_thread_list=0x00007f30a8002610, length=13, elements={
> 0x00007f30ec025690, 0x00007f30ec0b3d30, 0x00007f30ec0b5ae0, 0x00007f30ec0baa40,
> 0x00007f30ec0bbdf0, 0x00007f30ec0bd200, 0x00007f30ec0bebb0, 0x00007f30ec0c00e0,
> 0x00007f30ec0c1550, 0x00007f30ec0c9350, 0x00007f30ec0cc3d0, 0x00007f30ec0e5c20,
> 0x00007f30a8001650
> }
> 
> "main" #1 prio=5 os_prio=0 cpu=2.19ms elapsed=12.30s tid=0x00007f30ec025690 nid=0x69b0 in Object.wait()  [0x00007f30f392f000]
>    java.lang.Thread.State: WAITING (on object monitor)
> 	at java.lang.Object.wait(java.base at 17-internal/Native Method)
> 	- waiting on <0x00000000ca413e78> (a jdk.internal.crac.JDKContext)
> 	at java.lang.Object.wait(java.base at 17-internal/Object.java:338)
> 	at jdk.crac.impl.AbstractContextImpl.waitWhileCheckpointIsInProgress(java.base at 17-internal/AbstractContextImpl.java:102)
> 	at jdk.crac.impl.PriorityContext.register(java.base at 17-internal/PriorityContext.java:44)
> 	- locked <0x00000000ca413e78> (a jdk.internal.crac.JDKContext)
> 	at jdk.internal.crac.JDKContext.register(java.base at 17-internal/JDKContext.java:108)
> 	at jdk.internal.ref.CleanerImpl$PhantomCleanableRef.<init>(java.base at 17-internal/CleanerImpl.java:173)
> 	at java.lang.invoke.MethodHandleNatives$CallSiteContext.make(java.base at 17-internal/MethodHandleNatives.java:93)
> 	at java.lang.invoke.CallSite.<init>(java.base at 17-internal/CallSite.java:144)
> 	at java.lang.invoke.ConstantCallSite.<init>(java.base at 17-internal/ConstantCallSite.java:50)
> 	at java.lang.invoke.InnerClassLambdaMetafactory.buildCallSite(java.base at 17-internal/InnerClassLambdaMetafactory.java:262)
> 	at java.lang.invoke.LambdaMetafactory.metafactory(java.base at 17-internal/LambdaMetafactory.java:341)
> 	at java.lang.invoke.DirectMethodHandle$Holder.invokeStatic(java.base at 17-internal/DirectMethodHandle$Holder)
> 	at java.lang.invoke.Invokers$Holder.invokeExact_MT(java.base at 17-internal/Invokers$Holder)
> 	at java.lang.invoke.BootstrapMethodInvoker.invoke(java.base at 17-internal/BootstrapMethodInvoker.java:134)
> 	at java.lang.invoke.CallSite.makeSite(java.base at 17-internal/CallSite.java:315)
> 	at java.lang.invoke.MethodHandleNatives.linkCallSiteImpl(ja...

LGTM, except few nits.

src/java.base/share/classes/jdk/crac/Core.java line 178:

> 176: 
> 177:         if (newProperties != null && newProperties.length > 0) {
> 178:             Arrays.stream(newProperties).map(new Function<String, String[]>() {

A comment reffering to the issue and describing the solution (do not use lambda) would be helpful!

test/jdk/jdk/crac/CracOptionTest.java line 57:

> 55:         directory = Paths.get(System.getProperty("user.dir"), "workdir");
> 56:         directory.toFile().mkdir();
> 57:         Files.createTempFile(directory, "temp", ".txt");

Are these lines are required for the test? It looks that the rest of the test passes -Dk=v to both checkpoint and restore, triggering the issue. But these seems not to be related.

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

PR Review: https://git.openjdk.org/crac/pull/75#pullrequestreview-1443386078
PR Review Comment: https://git.openjdk.org/crac/pull/75#discussion_r1205194337
PR Review Comment: https://git.openjdk.org/crac/pull/75#discussion_r1205197191