[crac] RFR: 8349819: [CRaC] Support FD policy reopen on listening socket

Timofei Pushkin tpushkin at openjdk.org
Tue Feb 18 18:54:15 UTC 2025


On Tue, 18 Feb 2025 15:08:33 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> src/java.base/linux/classes/sun/nio/ch/EPollPort.java line 99:
>> 
>>> 97:     private class CRaCResource implements JDKResource {
>>> 98:         private volatile Phaser phaser;
>>> 99:         private AtomicInteger counter;
>> 
>> Shouldn't the counter also be volatile to ensure the assignment of a new `AtomicInteger` object is properly synchronized?
>
> Please check my reasoning (probably deserves a comment in code): `processCheckpoint()` is invoked only after `queue.take()` receiving `CHECKPOINT` event. The communication through queue establishes a happens-after relation (read-after-write barrier or what is this called?), hence the `volatile` is not needed. However `phaser` is accessed from `isCheckpoint()` after `poll()` returning `EXECUTE_TASK_OR_SHUTDOWN` as the `wakeup()` was issued. I don't see anything that would guarantee the happens-after, therefore it's rather `volatile`.
> 
> Might be worth to just add a couple of `synchronized` and don't mess with the fragile details.

Your reasoning seems correct, I didn't consider that the queue entails happens-before guarantees. A comment is definitely needed here.

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

PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1960074738


More information about the crac-dev mailing list