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

Alexey Bakhtin abakhtin at openjdk.java.net
Thu Jan 13 10:52:59 UTC 2022


On Thu, 13 Jan 2022 05:26:59 GMT, Anton Kozlov <akozlov at openjdk.org> wrote:

>> 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.

I do not like to replace LOCK_GET_BYTES with crLock. It will affect performance because of NativePRNG is singleton and the original LOCK_GET_BYTES synchronizes much smaller pieces of code.
The only standard implementation of ReadWriteLock is ReentrantReadWriteLock. So I think it is OK to use ReentrantReadWriteLock with additional checks I just proposed

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

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


More information about the crac-dev mailing list