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

[Timofei Pushkin]

On Fri, 14 Feb 2025 16:58:51 GMT, Radim Vansa <rvansa 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?

src/java.base/linux/classes/sun/nio/ch/EPollPort.java line 130:

> 128:             }
> 129:             phaser.arriveAndAwaitAdvance();
> 130:             phaser.arriveAndAwaitAdvance();

I think short comments on each `arriveAndAwaitAdvance` would simplify understanding of the code, e.g. "Await checkpoint", "Await restore"...

src/java.base/share/classes/java/net/SocketImpl.java line 78:

> 76: 
> 77:     @SuppressWarnings("unused")
> 78:     private final JDKSocketResource resource = new JDKSocketResource(SocketImpl.this) {

Isn't `this` the same as `SocketImpl.this` in this scope?

src/java.base/share/classes/jdk/internal/crac/JDKSocketResource.java line 41:

> 39:         Predicate<Map<String, String>> listenMatcher = params -> {
> 40:             String cfgListening = params.get("listening");
> 41:             if (cfgListening == null || "*".equals(cfgListening)) {

To me `listening: *` does not seem as good as for address and port. Maybe just `cfgListening == null` would be enough? `family: *` is not allowed, IIRC, so not allowing `listening: *` won't be inconsistent.

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.

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

PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1959703856
PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1959675687
PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1959713084
PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1959787327
PR Review Comment: https://git.openjdk.org/crac/pull/204#discussion_r1959743029