From cfff726054597ef24c2602395cdd5e8c0d749a79 Mon Sep 17 00:00:00 2001
From: user <user@inference.local>
Date: Fri, 4 Sep 2026 05:33:07 +0000
Subject: [PATCH] fix(ircserver): KILL notify-and-close must not block the
 killer

WIP: Disconnect ran two blocking writes to the victim's socket on the
killer's goroutine with the full 30s writeTimeout each, so a victim that
stopped reading stalled the killer up to ~60s -- wedging the operator's
serve() loop or the HTTP KILL request. Move the notify-and-close to its
own goroutine and bound both writes with a short killWriteWindow.
---
 internal/ircserver/conn.go | 57 ++++++++++++++++++++++++++++++++------
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/internal/ircserver/conn.go b/internal/ircserver/conn.go
index e448afb..06060b6 100644
--- a/internal/ircserver/conn.go
+++ b/internal/ircserver/conn.go
@@ -22,6 +22,7 @@ const (
 	maxLineLen      = 512
 	readTimeout     = 5 * time.Minute
 	writeTimeout    = 30 * time.Second
+	killWriteWindow = 2 * time.Second
 	dnsTimeout      = 3 * time.Second
 	pollInterval    = 100 * time.Millisecond
 	pingInterval    = 90 * time.Second
@@ -112,6 +113,18 @@ func newConn(
 // victim would keep a socket that looks alive but silently
 // delivers nothing.  Disconnect is called from the killer's
 // goroutine, never the victim's.
+//
+// The notify-and-close half runs on its own goroutine and
+// under a short deadline.  There is no per-client send
+// queue: send() writes straight to the victim's socket, so
+// a victim that has stopped reading would otherwise stall
+// the killer for the full writeTimeout on each of the two
+// writes -- wedging the operator's own serve() loop on the
+// IRC path, or the API request on the HTTP path.  Any
+// client could trigger that deliberately.  Disconnect
+// therefore returns as soon as the victim is marked closed;
+// the socket is closed shortly afterwards regardless of
+// whether the notification could be delivered.
 func (c *Conn) Disconnect(reason string) {
 	c.mu.Lock()
 
@@ -131,19 +144,36 @@ func (c *Conn) Disconnect(reason string) {
 		nick = "*"
 	}
 
-	c.sendFromServer(irc.CmdKill, nick, reason)
-	c.send(
-		"ERROR :Closing Link: " + host +
-			" (" + reason + ")",
-	)
-
 	// Stop the relay goroutine, which would otherwise keep
 	// polling a queue belonging to a deleted session.
 	if c.cancel != nil {
 		c.cancel()
 	}
 
-	c.conn.Close() //nolint:errcheck,gosec
+	go c.notifyKilledAndClose(nick, host, reason)
+}
+
+// notifyKilledAndClose delivers the KILL and ERROR lines to
+// a killed victim and then closes its socket.  It runs on a
+// goroutine owned by neither the killer nor the victim, and
+// bounds both writes with killWriteWindow so an unresponsive
+// victim cannot hold the socket open indefinitely.
+func (c *Conn) notifyKilledAndClose(
+	nick, host, reason string,
+) {
+	defer c.conn.Close() //nolint:errcheck,gosec
+
+	c.sendWithin(
+		killWriteWindow,
+		FormatMessage(
+			c.serverSfx, irc.CmdKill, nick, reason,
+		),
+	)
+	c.sendWithin(
+		killWriteWindow,
+		"ERROR :Closing Link: "+host+
+			" ("+reason+")",
+	)
 }
 
 // buildCommandMap returns a map from IRC command strings
@@ -285,11 +315,22 @@ func (c *Conn) currentNick() string {
 
 // send writes a formatted IRC line to the connection.
 func (c *Conn) send(line string) {
+	c.sendWithin(writeTimeout, line)
+}
+
+// sendWithin writes a formatted IRC line to the connection
+// under the given write deadline.  Callers that must not be
+// held hostage by an unresponsive peer pass a shorter window
+// than writeTimeout.
+func (c *Conn) sendWithin(
+	timeout time.Duration,
+	line string,
+) {
 	c.writeMu.Lock()
 	defer c.writeMu.Unlock()
 
 	_ = c.conn.SetWriteDeadline(
-		time.Now().Add(writeTimeout),
+		time.Now().Add(timeout),
 	)
 
 	_, _ = fmt.Fprintf(c.conn, "%s\r\n", line)