[crac] RFR: Fix ordering of invocation on Resources
Radim Vansa
duke at openjdk.org
Mon Apr 24 07:02:19 UTC 2023
On Fri, 21 Apr 2023 17:14:13 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:
>> * When Context.beforeCheckpoint throws, invoke Context.afterRestore anyway (otherwise some resources stay in suspended state).
>> * Handle Resource.beforeCheckpoint triggering a registration of another resource ** Do not cause deadlock when registering from another thread ** Global resource can register JDKResource
>> ** JDKResource can register resource with higher priority ** Other registrations are prohibited
>
> src/java.base/share/classes/jdk/crac/impl/AbstractContextImpl.java line 54:
>
>> 52: locked = true;
>> 53: // This is important for the case of recursive registration
>> 54: throwIfCheckpointInProgress(priority);
>
> What is expected handling of this exception? And in the current form the exception is not checked, so in most cases that exception won't be expected. Having that in most cases resources are registered during class or object initialization, those entities will be caught in partially constructed states, likely leaving the whole system stuck.
>
> Would it not better to allow registration, but throw CheckpointException at the end of Context.beforeCheckpoint? That is pretty legit exception that means the checkpoint can be attempted again, and another attempt can be pretty well successful, if all Resources do not throw and no new Resources are registered.
So, making `register` throw a checked exception wouldn't be a good solution; most of the time there wouldn't be a better handling than to log && rethrow.
What you propose - having a form of exception stack in the context and checking it anything was added after `beforeCheckpoint` could work, though this is definitely not a pattern that would be natural in Java - it smells of JNI developer :)
I think that a situation when the ISE is thrown is a programming error, rather than reacting on the state of 'outer world' (as in case of e.g. I/O...) and as such this can stay as unchecked exception, possibly leading to inconsistent state of the program.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/60#discussion_r1174861133
More information about the crac-dev
mailing list