Retry VNC accept on transient connection and resource errors

client/vnc/server/server.go

@@ -12,6 +12,7 @@ import (
 	"io"
 	"net"
 	"net/netip"
+	"slices"
 	"sync"
 	"syscall"
 	"time"
@@ -1295,13 +1296,26 @@ func approvalRejection(err error) (string, string) {
 }
 
 // acceptRetryable reports whether an Accept error can plausibly clear on its
-// own. Running out of file descriptors can, once open connections close, and an
-// aborted handshake concerns one client rather than the listener. Anything else
-// will fail the same way on every call, and retrying it is not recovery but a
-// livelock: Android has been seen returning EINVAL from accept4 for the life of
-// an otherwise healthy listening socket.
+// own: a pending connection that died before it could be handed over, or a
+// resource shortage that eases as open connections close and memory frees.
+// Both concern one attempt rather than the listening socket.
+//
+// The set is an allowlist rather than a denylist of fatal errors, and it
+// deliberately does not use net.Error.Temporary: an error that recurs on every
+// call, such as EINVAL from a listening socket whose interface went away, is
+// reported as temporary there and would turn the loop into a livelock instead
+// of a recovery.
 func acceptRetryable(err error) bool {
-	return errors.Is(err, syscall.ECONNABORTED) ||
-		errors.Is(err, syscall.EMFILE) ||
-		errors.Is(err, syscall.ENFILE)
+	retryable := []syscall.Errno{
+		syscall.ECONNABORTED, // the pending connection was reset during the handshake
+		syscall.ECONNRESET,   // same, reported differently by some stacks
+		syscall.ETIMEDOUT,    // the pending connection timed out before being handed over
+		syscall.EMFILE,       // per-process descriptor limit
+		syscall.ENFILE,       // system-wide descriptor limit
+		syscall.ENOBUFS,      // socket buffer memory exhausted
+		syscall.ENOMEM,       // kernel memory exhausted
+	}
+	return slices.ContainsFunc(retryable, func(errno syscall.Errno) bool {
+		return errors.Is(err, errno)
+	})
 }

client/vnc/server/server_test.go

@@ -15,6 +15,7 @@ import (
 	"net/netip"
 	"strings"
 	"sync/atomic"
+	"syscall"
 	"testing"
 	"time"
 
@@ -719,3 +720,32 @@ func TestCloseAdmission_HandlerOutlivingDrain(t *testing.T) {
 		t.Fatal("drain signal never fired after restart")
 	}
 }
+
+func TestAcceptRetryable(t *testing.T) {
+	tests := []struct {
+		name      string
+		err       error
+		retryable bool
+	}{
+		{"aborted handshake", syscall.ECONNABORTED, true},
+		{"reset pending connection", syscall.ECONNRESET, true},
+		{"timed out pending connection", syscall.ETIMEDOUT, true},
+		{"process fd limit", syscall.EMFILE, true},
+		{"system fd limit", syscall.ENFILE, true},
+		{"no socket buffers", syscall.ENOBUFS, true},
+		{"out of kernel memory", syscall.ENOMEM, true},
+		// A retryable errno reached through the layers Accept actually wraps it in.
+		{"wrapped in OpError", &net.OpError{Op: "accept", Err: syscall.ECONNABORTED}, true},
+		// EINVAL persists for the life of a socket whose interface is gone, so
+		// retrying it livelocks. net.Error.Temporary would call it temporary.
+		{"invalid listening socket", syscall.EINVAL, false},
+		{"listener closed", net.ErrClosed, false},
+		{"unrelated error", errors.New("boom"), false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.retryable, acceptRetryable(tt.err))
+		})
+	}
+}