From e4eb0c07010a32db3cf68083b2d63f9ceb21e009 Mon Sep 17 00:00:00 2001
From: Jeff Halter <868228+jhalter@users.noreply.github.com>
Date: Thu, 9 Jul 2026 18:42:32 -0700
Subject: Force-close accepted connections and drain session goroutines on
 shutdown

Canceling the server context closed only the listeners: sessions and
file transfers never observed cancellation, so their goroutines kept
running after ListenAndServe returned, blocked in reads until the
remote side went away. Nothing joined them either, so shutdown raced
whatever work was still in flight.

The server now tracks every accepted connection (sessions and file
transfers) in a registry. After the serve loops stop, ListenAndServe
force-closes the tracked connections, which unblocks their read loops,
and waits on a WaitGroup covering every session, file transfer, and
client writer goroutine before returning. Connections that race in
after shutdown begins are closed on arrival. The registry initializes
lazily so test-constructed Servers keep working.

The 3-second Windows close workaround in handleFileTransfer is skipped
when the context is canceled so in-flight transfers do not delay
process exit, and the transfer goroutine no longer assigns its error
to the accept loop's captured variable.
---
 hotline/server_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

(limited to 'hotline/server_test.go')

diff --git a/hotline/server_test.go b/hotline/server_test.go
index a9842c8..fb9a9c7 100644
--- a/hotline/server_test.go
+++ b/hotline/server_test.go
@@ -1018,6 +1018,58 @@ func TestServer_ListenAndServe_returnsErrorWhenPortUnavailable(t *testing.T) {
 	assert.NotErrorIs(t, err, context.Canceled)
 }
 
+// TestServer_ListenAndServe_closesActiveConnsOnCancel verifies that shutdown force-closes
+// accepted connections rather than only closing the listeners: a client blocked mid-handshake and
+// a file transfer connection that never sends its header would otherwise keep their session
+// goroutines alive past ListenAndServe's return.
+func TestServer_ListenAndServe_closesActiveConnsOnCancel(t *testing.T) {
+	port := findFreePortPair(t)
+	srv, err := NewServer(
+		WithLogger(NewTestLogger()),
+		WithInterface("127.0.0.1"),
+		WithPort(port),
+	)
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	errCh := make(chan error, 1)
+	go func() { errCh <- srv.ListenAndServe(ctx) }()
+
+	// Give the listeners a moment to start.
+	time.Sleep(100 * time.Millisecond)
+
+	// A session connection that stalls mid-handshake and a file transfer connection that never
+	// sends its 16-byte header: both park their serving goroutines in blocking reads.
+	sessionConn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port))
+	require.NoError(t, err)
+	defer func() { _ = sessionConn.Close() }()
+
+	transferConn, err := net.Dial("tcp", fmt.Sprintf("127.0.0.1:%d", port+1))
+	require.NoError(t, err)
+	defer func() { _ = transferConn.Close() }()
+
+	// Give the accept loops a moment to hand the connections to their goroutines.
+	time.Sleep(100 * time.Millisecond)
+
+	cancel()
+
+	select {
+	case <-errCh:
+	case <-time.After(2 * time.Second):
+		t.Fatal("ListenAndServe did not return after context cancellation with active connections")
+	}
+
+	// Both connections must have been closed by the server: reads unblock with EOF.
+	for _, conn := range []net.Conn{sessionConn, transferConn} {
+		require.NoError(t, conn.SetReadDeadline(time.Now().Add(2*time.Second)))
+		_, err = conn.Read(make([]byte, 1))
+		assert.Error(t, err)
+		assert.NotErrorIs(t, err, os.ErrDeadlineExceeded)
+	}
+}
+
 func TestServer_Shutdown_stopsListenAndServe(t *testing.T) {
 	srv, err := NewServer(
 		WithLogger(NewTestLogger()),
-- 
cgit