From 5b63fd27fea8f8d1e3ec359a1912107ab956b5d2 Mon Sep 17 00:00:00 2001
From: Bastiaan <17990055+bbassie@users.noreply.github.com>
Date: Sun, 13 Sep 2026 14:44:18 +0200
Subject: [PATCH] Fix listener leaks when a control connection closes (#1027)

The close handler removed the log subscription transport from the global
lib logger, but updateLogSubscriptions adds it to the controller's
clusterLogger. Every control connection that subscribed to logs stayed
attached to clusterLogger after closing, keeping the connection alive and
running every later cluster log line through its filter.

handleDebugDumpWsRequest added a new debugEvents listener on each call
and overwrote ws_dumper, so the close handler only removed the last one.
Repeated requests stacked listeners, triggered MaxListenersExceededWarning
after 11 calls, and left the earlier listeners behind. Ignore the request
if the connection is already dumping.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 packages/controller/src/ControlConnection.ts |  5 +-
 test/controller/ControlConnection.test.js    | 55 +++++++++++++++++++-
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/packages/controller/src/ControlConnection.ts b/packages/controller/src/ControlConnection.ts
index b502429b..60f3e263 100644
--- a/packages/controller/src/ControlConnection.ts
+++ b/packages/controller/src/ControlConnection.ts
@@ -53,7 +53,7 @@ export default class ControlConnection extends BaseConnection {
 		});
 		this.connector.on("close", () => {
 			if (this.logTransport) {
-				logger.remove(this.logTransport);
+				this._controller.clusterLogger.remove(this.logTransport);
 				this.logTransport = null;
 			}
 			if (this.ws_dumper) {
@@ -1325,6 +1325,9 @@ export default class ControlConnection extends BaseConnection {
 	}
 
 	async handleDebugDumpWsRequest(request: lib.DebugDumpWsRequest) {
+		if (this.ws_dumper) {
+			return; // Already dumping
+		}
 		this.ws_dumper = data => {
 			if (this.connector.connected) {
 				this.send(new lib.DebugWsMessageEvent(data.direction, data.content));
diff --git a/test/controller/ControlConnection.test.js b/test/controller/ControlConnection.test.js
index 2ad0db9d..b97c5527 100644
--- a/test/controller/ControlConnection.test.js
+++ b/test/controller/ControlConnection.test.js
@@ -1,5 +1,10 @@
 import assert from "node:assert/strict";
-import { ControlConnection } from "@clusterio/controller";
+import events from "node:events";
+import * as lib from "@clusterio/lib";
+import { ControlConnection, ControllerHooks } from "@clusterio/controller";
+import { MockConnector, MockLogger } from "../mock.js";
+
+const addr = lib.Address.fromShorthand;
 
 describe("controller/src/ControlConnection", function() {
 	describe(".handleControllerRestartRequest()", function() {
@@ -38,4 +43,52 @@ describe("controller/src/ControlConnection", function() {
 			assert.equal(mockController.stopped, false);
 		});
 	});
+
+	describe("close cleanup", function() {
+		let connector;
+		let mockController;
+		let connection;
+
+		beforeEach(function() {
+			connector = new MockConnector(addr("controller"), addr({ controlId: 1 }));
+			connector._socket = {};
+			const transports = new Set();
+			mockController = {
+				hooks: new ControllerHooks(new MockLogger()),
+				router: null,
+				_registeredRequests: new Map(),
+				_fallbackedRequests: new Map(),
+				_registeredEvents: new Map(),
+				_snoopedEvents: new Map(),
+				subscriptions: { unsubscribeLink() {} },
+				clusterLogger: {
+					transports,
+					add(transport) { transports.add(transport); },
+					remove(transport) { transports.delete(transport); },
+				},
+				debugEvents: new events.EventEmitter(),
+				sendRequestToHostByInstanceId() {},
+			};
+			connection = new ControlConnection({ version: "test" }, connector, mockController, {}, 1);
+		});
+
+		it("removes the log transport from clusterLogger", async function() {
+			await connection.handleLogSetSubscriptionsRequest(
+				new lib.LogSetSubscriptionsRequest(true, false, [], [])
+			);
+			assert.equal(mockController.clusterLogger.transports.size, 1);
+			connector.emit("close");
+			assert.equal(mockController.clusterLogger.transports.size, 0);
+			assert.equal(connection.logTransport, null);
+		});
+
+		it("removes the debug dump listener after repeated requests", async function() {
+			await connection.handleDebugDumpWsRequest(new lib.DebugDumpWsRequest());
+			await connection.handleDebugDumpWsRequest(new lib.DebugDumpWsRequest());
+			assert.equal(mockController.debugEvents.listenerCount("message"), 1);
+			assert.equal(connector._socket.clusterio_ignore_dump, true);
+			connector.emit("close");
+			assert.equal(mockController.debugEvents.listenerCount("message"), 0);
+		});
+	});
 });