[crac] RFR: 8349819: [CRaC] Support FD policy reopen on listening socket
Radim Vansa
rvansa at openjdk.org
Tue Feb 18 18:54:15 UTC 2025
On Tue, 18 Feb 2025 13:01:48 GMT, Timofei Pushkin <tpushkin at openjdk.org> wrote:
>> Adds additional filtering option for `type: SOCKET` in OpenResourcePolicies: `listening: true` which is true for common server sockets.
>> With `action: reopen` these implement automatic close on checkpoint and reopening on restore, using the same local address (this obviously won't work well if the restore happens on a machine that does not have the IP used before checkpoint).
>
> 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.
> src/java.base/share/classes/sun/nio/ch/AsynchronousServerSocketChannelImpl.java line 172:
>
>> 170: }
>> 171:
>> 172: protected void implReopen() {
>
> Shouldn't this be abstract? It will probably be more common for implementors to need to take some action to reopen.
I will read this as 'we need to run tests for these on Windows as well (with simengine)'
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1959943461
PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1959910802
More information about the crac-dev
mailing list