NioDomainSocketChannel deletes the socket file when closed

[dop251]

Expected behavior

When running a NioServerDomainSocketChannel based server the expectation is that multiple clients can connect to it for as long as the server socket remains open.

Actual behavior

After the first client connection is closed by the server, the socket file gets deleted thus preventing any further connections.

Note that in order to get this bug triggered, the localAddress() has to be called at least once before the channel is closed (see the comment in the reproducer code).

I believe two changes are needed:

1. NioDomainSocketChannel.doClose() should not call NioDomainSocketUtil.deleteSocketFile(local)
2. In NioServerDomainSocketChannel.doClose(), the call to localAddress() should be placed before javaChannel().close().

Steps to reproduce

Run the test below

Minimal yet complete reproducer code (or URL to code)

import io.netty.bootstrap.ServerBootstrap;
import io.netty.channel.*;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.channel.socket.nio.NioDomainSocketChannel;
import io.netty.channel.socket.nio.NioServerDomainSocketChannel;
import org.junit.jupiter.api.Test;

import java.net.StandardProtocolFamily;
import java.net.UnixDomainSocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.SocketChannel;

public class NettyNioServerDomainSocketChannelTests {

    private static final String SOCK_PATH = "./test.sock";

    @Test
    public void test() throws Exception {

        class Handler extends SimpleChannelInboundHandler<Object> {

            @Override
            protected void channelRead0(ChannelHandlerContext ctx, Object msg) throws Exception {
                // The following is essential due to another problem: the java channel is closed
                // before the call to localAddress() (see https://github.com/netty/netty/blob/234b9e5b42ccb445bba090ca7a7d37d0d4a244e1/transport/src/main/java/io/netty/channel/socket/nio/NioDomainSocketChannel.java#L348)
                // If this happens to be the first call to localAddress(), null is returned and the socket file
                // never gets deleted.
                ctx.channel().localAddress();

                ctx.close();
            }
        }

        final Result<Channel> serverChannel = new Result<>();

        new Thread(() -> {
            NioEventLoopGroup group = new NioEventLoopGroup(1);
            try {
                ServerBootstrap bootstrap = new ServerBootstrap()
                        .group(group, group)
                        .channel(NioServerDomainSocketChannel.class)
                        .childHandler(new ChannelInitializer<NioDomainSocketChannel>() {
                            @Override
                            protected void initChannel(NioDomainSocketChannel ch) {
                                ch.pipeline().addLast(new Handler());
                            }
                        });
                ChannelFuture future = bootstrap.bind(UnixDomainSocketAddress.of(SOCK_PATH)).sync();
                synchronized (serverChannel) {
                    serverChannel.set(future.channel());
                    serverChannel.notifyAll();
                }
                future.channel().localAddress(); // same problem as above, without this line the socket file does not get deleted after the server channel is closed
                future.channel().closeFuture().sync();
            } catch (Exception e) {
                synchronized (serverChannel) {
                    serverChannel.setException(e);
                    serverChannel.notifyAll();
                }
            } finally {
                group.shutdownGracefully();
            }
        }).start();

        synchronized (serverChannel) {
            while (serverChannel.get() == null) {
                serverChannel.wait();
            }
        }

        try (SocketChannel channel = SocketChannel
                .open(StandardProtocolFamily.UNIX)) {
            channel.connect(UnixDomainSocketAddress.of(SOCK_PATH));
            ByteBuffer buffer = ByteBuffer.allocate(5);
            buffer.clear().put("hello".getBytes()).flip();
            while (buffer.hasRemaining()) {
                channel.write(buffer);
            }
            while (true) {
                buffer.clear();
                var r = channel.read(buffer);
                if (r == -1) {
                    break;
                }
            }
        }

        Thread.sleep(200); // give it time to delete the socket file

        // The second connection fails with "java.net.SocketException: No such file or directory"
        // because the socket file has been deleted.
        try (SocketChannel channel = SocketChannel
                .open(StandardProtocolFamily.UNIX)) {
            channel.connect(UnixDomainSocketAddress.of(SOCK_PATH));
        } finally {
            serverChannel.get().close();
        }
    }

    private static class Result<T> {
        T value;
        Exception e;

        T get() throws Exception {
            if (e != null) {
                throw e;
            }
            return value;
        }

        void set(T value) {
            this.value = value;
        }

        void setException(Exception e) {
            this.e = e;
        }
    }
}

Netty version

4.1.111.Final

JVM version (e.g. java -version)

openjdk version "21.0.3" 2024-04-16
OpenJDK Runtime Environment (build 21.0.3+9-Ubuntu-1ubuntu122.04.1)
OpenJDK 64-Bit Server VM (build 21.0.3+9-Ubuntu-1ubuntu122.04.1, mixed mode, sharing)

OS version (e.g. uname -a)

Linux dop-home 5.15.0-112-generic #122-Ubuntu SMP Thu May 23 07:48:21 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

[chrisvest]

Yeah, looks like a bug in NioDomainSocketChannel.
FYI @honglh

[dop251]

I have a draft PR with the fix, which branch should I submit it against, 4.1 or 4.2? Also, why is mailing address mandatory on the CLA? What if I prefer not give one?

[normanmaurer]

I have a draft PR with the fix, which branch should I submit it against, 4.1 or 4.2?
4.1

Also, why is mailing address mandatory on the CLA? What if I prefer not give one?
This is just how we are able to contact people if we would ever need.

[dop251]

Also, why is mailing address mandatory on the CLA? What if I prefer not give one?
This is just how we are able to contact people if we would ever need.

But that's not email (there is a separate field for it), which makes it look like a postal address. Do you send people postcards? :)

[normanmaurer]

Fixed by #14134