[crac] RFR: Provide arguments for restore [v5]

Dan Heidinga heidinga at openjdk.java.net
Mon Mar 21 20:15:57 UTC 2022


On Mon, 21 Mar 2022 18:56:00 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/crac/Core.java line 177:
>> 
>>> 175:                          InvocationTargetException |
>>> 176:                          NoSuchMethodException     |
>>> 177:                          IllegalAccessException e) {
>> 
>> Thanks for adapting this to be more specific on what it catches.
>> 
>> I don't think it's quite right yet though as the current code will allow exceptions thrown by the `newMain` method (and its callees) to propagate past the checkpoint spot and makes the caller of the checkpoint code need to handle them.  
>> 
>> At the time `Core.checkpointRestore();` is called, we may have stack that looks llike:
>> 
>> TOS
>> Core.checkpointRestore();
>> Foo.bar();
>> Foo.foobar();
>> SomeOtherClass.method();
>> OriginalClass.main();
>> 
>> 
>> And when we restore, we load an execute a new `main()` method as though it was called where `Core.checkpointRestore();` was previously on the stack resulting in:
>> 
>> TOS
>> newMain.main();
>> Core.checkpointRestore();
>> Foo.bar();
>> Foo.foobar();
>> SomeOtherClass.method();
>> OriginalClass.main();
>> 
>> 
>> So exceptions thrown by code called from `newMain` should not propagate past `Core.checkpointRestore();` without being wrapped in a `RestoreException`.  `Error`-subclasses should propagate.
>> 
>> I think the code should be refactored to something like:
>> 
>>                } catch(Exception t) {
>>                    assert checkpointException == null :
>>                         "should not have new arguments";
>>                     if (restoreException == null) {
>>                         restoreException = new RestoreException();
>>                     }
>>                     restoreException.addSuppressed(e);
>>                 }
>> 
>> as that correctly catches all Exceptions but lets the Errors propagate past
>
>> the current code will allow exceptions thrown by the `newMain` method (and its callees) to propagate past the checkpoint spot and makes the caller of the checkpoint code need to handle them.
> 
> An exception from the newMain will be wrapped in InvocationTargetException, which is caught and is suppressed by the RestoreException. Right? Otherwise, this java code won't compile, as checkpointRestore is declared to throw only {Checkpoint,Restore}Exception.

I had to double check with `jshell` as I thought runtime exceptions would propagate past.  Turns out your 100% right about the all Exceptions/Errors/Throwables being wrapped with `InvocationTargetException`.

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

PR: https://git.openjdk.java.net/crac/pull/16


More information about the crac-dev mailing list