listeners: close unix socket immediately and unlink file on reload (#8061)

Co-authored-by: Zen Dodd <mail@steadytao.com>

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 {

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()
+	}
+}