[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