[crac] RFR: Reseed NativePRNG on checkpoint restore [v2]

[Anton Kozlov]

On Fri, 24 Dec 2021 21:00:22 GMT, Alexey Bakhtin <abakhtin at openjdk.org> wrote:

>> src/java.base/unix/classes/sun/security/provider/NativePRNG.java line 589:
>> 
>>> 587:         @Override
>>> 588:         public void beforeCheckpoint(Context<? extends Resource> context) throws Exception {
>>> 589:             crLock.lock();
>> 
>> The new lock is very related to `LOCK_GET_BYTES`, however, this code does not acquire `LOCK_GET_BYTES`. Now all `synchronized (LOCK_GET_BYTES)` blocks are executed under `crLock`. But I would prefer at least `assert crLock.isHeldByCurrentThread()` near those synchronized blocks, or any other way to ensure / document locks relation. Or use the LOCK_GET_BYTES, considering the other problem with premature buffer and mixRandom initialzation.
>
> I do not see any issues between `crLock` and `LOCK_GET_BYTES.` `beforeCheckpoint` does not acquire any additional locks, so it should not cause deadlocks.
> It is normal if different threads try to acquire `crLock.` In this case, one of them waits for the completion of another. E.g. `beforeCheckpoint` waits for completion `implNextBytes()` and vice versa. So, assert `crLock.isHeldByCurrentThread()` is not required.
> In case of `implNextBytes` called from the `beforeCheckpoint` and `crLock` already acquired by `RandomIO.beforeCheckpoint` will have improper priorities of dependent `JDKResources.` It should be properly adjusted in the `JDKResource.Priority` (see #8)
> The only problem is performance. `INSTANCE` is a single object. So, locking the whole  `implNextBytes` method will affect performance dramatically. I think It can be fixed by `ReentrantReadWriteLock`. Will update implementation

crLock and LOCK_GET_BYTES should be acquired in the particular order to ensure no deadlock can happen. The order is correct now AFAICS, but in the code these locks are used in distant places, checking the locks relation is nontrivial. Someone in the future may add another use of LOCK_GET_BYTES that would go out-of-sync with crLock.

I support using a ReadWriteLock. I'm not sure about how bad the performance was without one (the LOCK_GET_BYTES is there anyway). But some ReadWriteLock is certainly better suited for our task. I'm still not sure the ReentrantReadWriteLock is a perfect one, see another thread.

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

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