[crac] RFR: Fix ordering of invocation on Resources [v3]

[Radim Vansa]

On Fri, 28 Apr 2023 11:59:17 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

>> Radim Vansa has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - More fine-grained synchronization
>>  - Rework context ordering (round 2)
>>    
>>    * call afterRestore even if beforeCheckpoint throws
>>    * registering resource in previous/running context does not trigger exception immediatelly
>>    ** instead this will be one of the recorded exceptions and the resource has a chance to fire next time
>>    * we don't guarantee threads not deadlocking when trying to register a resource, though
>
> src/java.base/share/classes/jdk/crac/impl/AbstractContextImpl.java line 59:
> 
>> 57:             recordExceptions(e);
>> 58:         } catch (Exception e) {
>> 59:             Core.recordException(e);
> 
> Why is there is the distinction? I think we should throw all exceptions from the context, rather than publishing them to a central store, otherwise the parent Context (if any), won't be able to do anything about those.

The contexts are supposed to run all resources regardless of whether any failed, therefore there's no point in propagating and re-wrapping the exceptions. We could have a method to check whether this C/R is 'marked for rollback' (has any exceptions), but I don't see why would the parent context decide on the type - because it would get just CheckpointException with a list of other failures from components it does not understand.

About the distinction: the difference is that if you get a CheckpointException you'd unwrap it, recording only the inner suppressed ones. But I should push that to `recordExceptions` and rather decide based on CheckpointException message than number of suppressions.

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

PR Review Comment: https://git.openjdk.org/crac/pull/60#discussion_r1182150531