From d483d7816f7a28fc104a21a2e33f0723261fc758 Mon Sep 17 00:00:00 2001 From: Little Fedya Date: Sat, 3 Oct 2026 08:45:47 +0700 Subject: [PATCH] listeners: close unix socket immediately and unlink file on reload (#8061) Co-authored-by: Zen Dodd --- listen_unix.go | 96 +++++++---- listen_unix_test.go | 408 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 468 insertions(+), 36 deletions(-) create mode 100644 listen_unix_test.go diff --git a/listen_unix.go b/listen_unix.go index d60f69f3b..51940661d 100644 --- a/listen_unix.go +++ b/listen_unix.go @@ -27,8 +27,10 @@ import ( "io/fs" "net" "os" + "runtime" "slices" "strconv" + "strings" "sync" "sync/atomic" "syscall" @@ -50,6 +52,7 @@ func reuseUnixSocket(network, addr string) (any, error) { if err != nil { return nil, err } + defer socketFile.Close() // use copied fd to make new Listener or PacketConn, then replace // it in the map so that future copies always come from the most @@ -64,7 +67,11 @@ func reuseUnixSocket(network, addr string) (any, error) { return nil, err } unixSocket.count.Add(1) - unixSockets[socketKey] = &unixListener{ln.(*net.UnixListener), socketKey, unixSocket.count} + unixSockets[socketKey] = &unixListener{ + UnixListener: ln.(*net.UnixListener), + mapKey: socketKey, + count: unixSocket.count, + } case *unixConn: pc, err := net.FilePacketConn(socketFile) @@ -72,11 +79,18 @@ func reuseUnixSocket(network, addr string) (any, error) { return nil, err } unixSocket.count.Add(1) - unixSockets[socketKey] = &unixConn{pc.(*net.UnixConn), socketKey, unixSocket.count} + unixSockets[socketKey] = &unixConn{ + UnixConn: pc.(*net.UnixConn), + mapKey: socketKey, + count: unixSocket.count, + } } return unixSockets[socketKey], nil } + if isAbstractUnixSocket(addr) { + return nil, nil + } // from what I can tell after some quick research, it's quite common for programs to // leave their socket file behind after they close, so the typical pattern is to @@ -167,7 +181,7 @@ func listenReusable(ctx context.Context, lnKey string, network, address string, if unix, ok := ln.(*net.UnixConn); ok { cnt := new(atomic.Int32) cnt.Store(1) - ln = &unixConn{unix, lnKey, cnt} + ln = &unixConn{UnixConn: unix, mapKey: lnKey, count: cnt} unixSockets[lnKey] = ln.(*unixConn) } } @@ -184,7 +198,7 @@ func listenReusable(ctx context.Context, lnKey string, network, address string, unix.SetUnlinkOnClose(false) cnt := new(atomic.Int32) cnt.Store(1) - ln = &unixListener{unix, lnKey, cnt} + ln = &unixListener{UnixListener: unix, mapKey: lnKey, count: cnt} unixSockets[lnKey] = ln.(*unixListener) } } @@ -219,52 +233,62 @@ type unixListener struct { *net.UnixListener mapKey string count *atomic.Int32 + closed atomic.Bool } func (uln *unixListener) Close() error { - newCount := uln.count.Add(-1) - if newCount == 0 { - file, err := uln.File() - var name string - if err == nil { - name = file.Name() - } - defer func() { - unixSocketsMu.Lock() - delete(unixSockets, uln.mapKey) - unixSocketsMu.Unlock() - if err == nil { - _ = syscall.Unlink(name) - } - }() + if !uln.closed.CompareAndSwap(false, true) { + return net.ErrClosed } - return uln.UnixListener.Close() + unixSocketsMu.Lock() + defer unixSocketsMu.Unlock() + + var name string + if addr := uln.Addr(); addr != nil { + name = addr.String() + } + newCount := uln.count.Add(-1) + err := uln.UnixListener.Close() + if newCount == 0 { + delete(unixSockets, uln.mapKey) + if name != "" && !isAbstractUnixSocket(name) { + _ = syscall.Unlink(name) + } + } + return err } type unixConn struct { *net.UnixConn mapKey string count *atomic.Int32 + closed atomic.Bool } func (uc *unixConn) Close() error { - newCount := uc.count.Add(-1) - if newCount == 0 { - file, err := uc.File() - var name string - if err == nil { - name = file.Name() - } - defer func() { - unixSocketsMu.Lock() - delete(unixSockets, uc.mapKey) - unixSocketsMu.Unlock() - if err == nil { - _ = syscall.Unlink(name) - } - }() + if !uc.closed.CompareAndSwap(false, true) { + return net.ErrClosed } - return uc.UnixConn.Close() + unixSocketsMu.Lock() + defer unixSocketsMu.Unlock() + + var name string + if addr := uc.LocalAddr(); addr != nil { + name = addr.String() + } + newCount := uc.count.Add(-1) + err := uc.UnixConn.Close() + if newCount == 0 { + delete(unixSockets, uc.mapKey) + if name != "" && !isAbstractUnixSocket(name) { + _ = syscall.Unlink(name) + } + } + return err +} + +func isAbstractUnixSocket(name string) bool { + return (runtime.GOOS == "linux" || runtime.GOOS == "android") && strings.HasPrefix(name, "@") } func (uc *unixConn) Unwrap() net.PacketConn { diff --git a/listen_unix_test.go b/listen_unix_test.go new file mode 100644 index 000000000..5461da0bd --- /dev/null +++ b/listen_unix_test.go @@ -0,0 +1,408 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build unix && !solaris + +package caddy + +import ( + "context" + "errors" + "io" + "net" + "os" + "path/filepath" + "runtime" + "sync" + "testing" + "time" +) + +func TestAbstractUnixSocketsDoNotUnlinkFilesystemPaths(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("abstract Unix sockets are Linux-specific") + } + + for _, network := range []string{"unix", "unixgram"} { + t.Run(network, func(t *testing.T) { + sentinel, err := os.CreateTemp(".", "@caddy-abstract-*") + if err != nil { + t.Fatal(err) + } + name := filepath.Base(sentinel.Name()) + if err := sentinel.Close(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Remove(name) }) + + ln, err := (NetworkAddress{Network: network, Host: name}).Listen( + context.Background(), 0, net.ListenConfig{}) + if err != nil { + t.Fatal(err) + } + closed := false + t.Cleanup(func() { + if !closed { + _ = ln.(io.Closer).Close() + } + }) + if _, err := os.Stat(name); err != nil { + t.Fatalf("abstract listen removed sentinel: %v", err) + } + err = ln.(io.Closer).Close() + closed = true + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(name); err != nil { + t.Fatalf("abstract close removed sentinel: %v", err) + } + }) + } +} + +func TestUnixListenerDoubleCloseDoesNotReleaseAnotherReference(t *testing.T) { + socketPath := shortTempSocket(t) + na := NetworkAddress{Network: "unix", Host: socketPath} + + firstAny, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + t.Fatal(err) + } + firstOuter := firstAny.(deleteListener) + first := firstOuter.Listener.(*unixListener) + var second *unixListener + t.Cleanup(func() { + _ = firstOuter.Close() + if second != nil { + _ = second.Close() + } + }) + secondAny, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + t.Fatal(err) + } + second = secondAny.(*unixListener) + + if err := first.Close(); err != nil { + t.Fatal(err) + } + if err := first.Close(); !errors.Is(err, net.ErrClosed) { + t.Fatalf("second Close() error = %v; want net.ErrClosed", err) + } + if got := first.count.Load(); got != 1 { + t.Fatalf("reference count = %d; want 1", got) + } + if _, err := os.Stat(socketPath); err != nil { + t.Fatalf("socket used by second reference was removed: %v", err) + } + + if err := second.Close(); err != nil { + t.Fatal(err) + } +} + +func TestUnixConnDoubleCloseDoesNotReleaseAnotherReference(t *testing.T) { + socketPath := shortTempSocket(t) + na := NetworkAddress{Network: "unixgram", Host: socketPath} + + firstAny, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + t.Fatal(err) + } + firstOuter := firstAny.(deletePacketConn) + first := firstOuter.PacketConn.(*unixConn) + var second *unixConn + t.Cleanup(func() { + _ = firstOuter.Close() + if second != nil { + _ = second.Close() + } + }) + secondAny, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + t.Fatal(err) + } + second = secondAny.(*unixConn) + + if err := first.Close(); err != nil { + t.Fatal(err) + } + if err := first.Close(); !errors.Is(err, net.ErrClosed) { + t.Fatalf("second Close() error = %v; want net.ErrClosed", err) + } + if got := first.count.Load(); got != 1 { + t.Fatalf("reference count = %d; want 1", got) + } + if _, err := os.Stat(socketPath); err != nil { + t.Fatalf("socket used by second reference was removed: %v", err) + } + + if err := second.Close(); err != nil { + t.Fatal(err) + } +} + +// shortTempSocket returns a short unix socket path to avoid sockaddr_un +// path length limits (104 bytes on Darwin/BSD, 108 bytes on Linux), +// which can be exceeded by t.TempDir() in deeply nested temp directories. +func shortTempSocket(t *testing.T) string { + t.Helper() + f, err := os.CreateTemp("", "c-*.sock") + if err != nil { + t.Fatalf("failed to create temp socket path: %v", err) + } + path := f.Name() + _ = f.Close() + _ = os.Remove(path) + t.Cleanup(func() { + _ = os.Remove(path) + }) + return path +} + +func TestUnixListenerClosesImmediatelyAndUnlinks(t *testing.T) { + socketPath := shortTempSocket(t) + lnKey := listenerKey("unix", socketPath) + + ctx := context.Background() + rawLn, err := listenReusable(ctx, lnKey, "unix", socketPath, net.ListenConfig{}) + if err != nil { + t.Fatalf("listenReusable failed: %v", err) + } + + ln, ok := rawLn.(net.Listener) + if !ok { + t.Fatalf("expected net.Listener, got %T", rawLn) + } + + // Verify socket file was created on disk + if _, err := os.Stat(socketPath); err != nil { + t.Fatalf("expected socket file to exist on disk: %v", err) + } + + // Verify we can connect and accept + done := make(chan struct{}) + go func() { + defer close(done) + conn, err := ln.Accept() + if err == nil { + _ = conn.Close() + } + }() + + clientConn, err := net.DialTimeout("unix", socketPath, 2*time.Second) + if err != nil { + t.Fatalf("failed to dial unix socket: %v", err) + } + _ = clientConn.Close() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for accept") + } + + // Close the listener + if err := ln.Close(); err != nil { + t.Fatalf("failed to close listener: %v", err) + } + + // The socket file must be removed immediately + if _, err := os.Stat(socketPath); !os.IsNotExist(err) { + t.Fatalf("expected socket file to be unlinked after Close, but stat returned: %v", err) + } + + // Dialing the closed socket path must fail immediately and not hang + dialErrChan := make(chan error, 1) + go func() { + c, err := net.DialTimeout("unix", socketPath, 500*time.Millisecond) + if err == nil { + _ = c.Close() + } + dialErrChan <- err + }() + + select { + case err := <-dialErrChan: + if err == nil { + t.Fatal("expected dial to fail on closed socket, but succeeded") + } + case <-time.After(1 * time.Second): + t.Fatal("dialing closed socket hung instead of failing immediately") + } +} + +func TestUnixConnClosesImmediatelyAndUnlinks(t *testing.T) { + socketPath := shortTempSocket(t) + lnKey := listenerKey("unixgram", socketPath) + + ctx := context.Background() + rawConn, err := listenReusable(ctx, lnKey, "unixgram", socketPath, net.ListenConfig{}) + if err != nil { + t.Fatalf("listenReusable failed: %v", err) + } + + closer, ok := rawConn.(io.Closer) + if !ok { + t.Fatalf("expected io.Closer, got %T", rawConn) + } + + // Verify socket file was created on disk + if _, err := os.Stat(socketPath); err != nil { + t.Fatalf("expected socket file to exist on disk: %v", err) + } + + // Close the datagram conn + if err := closer.Close(); err != nil { + t.Fatalf("failed to close unixgram connection: %v", err) + } + + // The socket file must be removed immediately + if _, err := os.Stat(socketPath); !os.IsNotExist(err) { + t.Fatalf("expected socket file to be unlinked after Close, but stat returned: %v", err) + } +} + +func TestUnixListenerReuseAndUnlinkOnlyWhenZeroCount(t *testing.T) { + socketPath := shortTempSocket(t) + lnKey := listenerKey("unix", socketPath) + + ctx := context.Background() + rawLn1, err := listenReusable(ctx, lnKey, "unix", socketPath, net.ListenConfig{}) + if err != nil { + t.Fatalf("listenReusable failed: %v", err) + } + ln1 := rawLn1.(net.Listener) + + // Reuse socket + rawLn2, err := reuseUnixSocket("unix", socketPath) + if err != nil { + t.Fatalf("reuseUnixSocket failed: %v", err) + } + ln2 := rawLn2.(net.Listener) + + // Close ln1: count drops to 1, socket must still exist + if err := ln1.Close(); err != nil { + t.Fatalf("ln1.Close() failed: %v", err) + } + if _, err := os.Stat(socketPath); err != nil { + t.Fatalf("expected socket file to still exist after closing 1 of 2 references: %v", err) + } + + // Close ln2: count drops to 0, socket file must be unlinked + if err := ln2.Close(); err != nil { + t.Fatalf("ln2.Close() failed: %v", err) + } + if _, err := os.Stat(socketPath); !os.IsNotExist(err) { + t.Fatalf("expected socket file to be unlinked after closing all references, got: %v", err) + } +} + +func TestUnixListenerConcurrentCloseAndRelisten(t *testing.T) { + socketPath := shortTempSocket(t) + na := NetworkAddress{Network: "unix", Host: socketPath} + + for i := 0; i < 20; i++ { + ln, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + t.Fatalf("iteration %d: initial listen failed: %v", i, err) + } + listener := ln.(net.Listener) + + var wg sync.WaitGroup + wg.Add(2) + + var newLn net.Listener + var listenErr error + + go func() { + defer wg.Done() + _ = listener.Close() + }() + + go func() { + defer wg.Done() + ln2, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + listenErr = err + return + } + newLn = ln2.(net.Listener) + }() + + wg.Wait() + + if listenErr != nil { + t.Fatalf("iteration %d: concurrent listen failed: %v", i, listenErr) + } + + if _, err := os.Stat(socketPath); err != nil { + t.Fatalf("iteration %d: socket file does not exist after concurrent relisten: %v", i, err) + } + + clientConn, err := net.DialTimeout("unix", socketPath, 500*time.Millisecond) + if err != nil { + t.Fatalf("iteration %d: dialing concurrent listener failed: %v", i, err) + } + _ = clientConn.Close() + + _ = newLn.Close() + } +} + +func TestUnixConnConcurrentCloseAndRelisten(t *testing.T) { + socketPath := shortTempSocket(t) + na := NetworkAddress{Network: "unixgram", Host: socketPath} + + for i := 0; i < 20; i++ { + pc, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + t.Fatalf("iteration %d: initial listen failed: %v", i, err) + } + packetConn := pc.(net.PacketConn) + + var wg sync.WaitGroup + wg.Add(2) + + var newPc net.PacketConn + var listenErr error + + go func() { + defer wg.Done() + _ = packetConn.Close() + }() + + go func() { + defer wg.Done() + pc2, err := na.Listen(context.Background(), 0, net.ListenConfig{}) + if err != nil { + listenErr = err + return + } + newPc = pc2.(net.PacketConn) + }() + + wg.Wait() + + if listenErr != nil { + t.Fatalf("iteration %d: concurrent listen failed: %v", i, listenErr) + } + + if _, err := os.Stat(socketPath); err != nil { + t.Fatalf("iteration %d: socket file does not exist after concurrent relisten: %v", i, err) + } + + _ = newPc.Close() + } +}