[crac] RFR: Introduce per-Priority Context with different policies

[Anton Kozlov]

On Thu, 18 May 2023 12:09:36 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> A follow-up work for #60:
>> 
>> * Each priority now has a dedicated context, so contextes may provide different policies. CALL_SITE now uses new CriticalUnorderedContext that runs beforeCheckpoint on concurrent registration, fixes [1]. Whether or not CALL_SITE needs to be registered to at all is an open question and out of scope of this PR.
>> * the Global Context reverted from BlockingOrderedContext to OrderedContext, as that may have a huge impact on users. Probably we'll want to expose blocking/criticalUnorderd context along the global one, or at some point expose an implementation. But this is also out of scope of the PR.
>> * hierachy of the Context implementations are cleaned up a bit [2]
>> 
>> The JDKContext is now just a holder of ClaimedFDs, I'll address this in a follow-up that depends on this Context follow-up.
>> 
>> [1] https://github.com/openjdk/crac/pull/60#issuecomment-1545588281
>> [2] https://github.com/openjdk/crac/pull/60#discussion_r1185510445
>
> src/java.base/share/classes/jdk/crac/impl/CriticalUnorderedContext.java line 83:
> 
>> 81:             if (concurrentCheckpointException != null) {
>> 82:                 try {
>> 83:                     invokeBeforeCheckpoint(resource);
> 
> This context is blocking as well - since you hold the monitor on this object if the code tries to indirectly (via operation in another thread) register other resource, it will deadlock, and cannot be woken up via interrupt.
> If the only usage is call sites we know that there won't be a recursive registration, so this not such a big flaw, though.

Right, this context implementation something that should suit JDK needs, and the point is to verify the user is capable to do something similar with existing API. So, indeed, there is a possiblity of the deadlock with other thread. Fixing this does not seem trivial, so let's assume JDK will have to do something about this, if the problem become possible.

> test/jdk/jdk/crac/ContextOrderTest.java line 106:
> 
>> 104: 
>> 105:         // blocks register into the same OrderedContext
>> 106:         Context<Resource> context = new BlockingOrderedContext<>();
> 
> Why does this create a children context rather than using global one directly?

Global Context was changed to non-blocking OrderedContext as stated in the PR description, but the test needs BlockingOrderedContext.

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

PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1198023550
PR Review Comment: https://git.openjdk.org/crac/pull/74#discussion_r1198024463