[crac] RFR: 8364449: [CRaC] Launch new main outside of the core C/R body [v2]

[Radim Vansa]

On Thu, 31 Jul 2025 16:41:20 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:

>> New main is now launched after restoration completes solving the issues outlined in the JBS task.
>> 
>> Also the error message when the new main is not found (e.g. the feature is used by accident like in https://github.com/spring-projects/spring-framework/issues/33226) or throws an exception should become less cryptic:
>> 
>> $ java -XX:CRaCRestoreFrom=cr -jar app.jar
>> Exception in thread "main" jdk.crac.RestoreException
>>         Suppressed: java.lang.Exception: Failed to execute the new main entry: new initial class = 'app.jar', new main arguments = []. Do not specify these if you just wish to continue the checkpointed execution.
>>                 at java.base/jdk.internal.crac.mirror.Core.checkpointRestore(Core.java:314)
>>                 at java.base/jdk.internal.crac.mirror.Core.checkpointRestore(Core.java:263)
>>                 at jdk.crac/jdk.crac.Core.checkpointRestore(Core.java:73)
>>                 at Main.main(Main.java:4)
>>         Caused by: java.lang.ClassNotFoundException: app.jar
>>                 at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:580)
>>                 at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:490)
>>                 at java.base/java.lang.Class.forName0(Native Method)
>>                 at java.base/java.lang.Class.forName(Class.java:547)
>>                 at java.base/jdk.internal.crac.mirror.Core.checkpointRestore(Core.java:302)
>>                 ... 3 more
>
> Timofei Pushkin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add missing space

Looks good, just a few minor remarks to code structure.

src/java.base/share/classes/jdk/internal/crac/mirror/Core.java line 226:

> 224:         restoreException.throwIfAny();
> 225: 
> 226:         final var parsedNewArguments = new ArrayList<String>();

`checkpointRestore1` is already a bit long; could you extract this to another method, please?

test/jdk/jdk/crac/newArgs/CheckpointInNewMainTest.java line 59:

> 57:         final var out = builder.doRestore().outputAnalyzer();
> 58: 
> 59:         out.shouldContain(RESTORE_OLD_MSG).shouldContain(RESTORE_NEW_MSG);

I think that a common pattern is to suppress continuation of the old main. Could you call add a variant that calls `System.exit(0)` in `InternalMain` and assert that `RESTORE_OLD_MSG` is not printed in that case?

test/jdk/jdk/crac/newArgs/FailedCheckpointRestoreTest.java line 52:

> 50: 
> 51:     public enum Variant {
> 52:         SUCCESS,

The `SUCCESS` variant looks identical to `CheckpointInNewMainTest`, except for registering a resource that wouldn't do anything. Could you merge these?

test/jdk/jdk/crac/newArgs/FailedCheckpointRestoreTest.java line 104:

> 102:             System.out.println(RESTORE_MSG);
> 103:         } catch (CheckpointException ex) {
> 104:             if (variant != Variant.CHECKPOINT_EXCEPTION) {

Could you rather check this using `jdk.test.lib.Asserts.*`?

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

PR Review: https://git.openjdk.org/crac/pull/253#pullrequestreview-3095585516
PR Review Comment: https://git.openjdk.org/crac/pull/253#discussion_r2259271706
PR Review Comment: https://git.openjdk.org/crac/pull/253#discussion_r2259254663
PR Review Comment: https://git.openjdk.org/crac/pull/253#discussion_r2259267289
PR Review Comment: https://git.openjdk.org/crac/pull/253#discussion_r2259263108