diff --git a/lib/core/mcp/mcp_server_controller.dart b/lib/core/mcp/mcp_server_controller.dart index ba95bc83..428b1fcd 100644 --- a/lib/core/mcp/mcp_server_controller.dart +++ b/lib/core/mcp/mcp_server_controller.dart @@ -99,7 +99,21 @@ class McpServerController { } } - Future start() async { + /// Runs [op] after every earlier start / stop has finished. `start` awaits + /// between its `_host != null` check and the assignment, so two overlapping + /// calls (a toggle clicked twice, `startIfEnabled` racing the settings page) + /// each built a host and the first one was leaked, still listening (#1563). + Future _lifecycle = Future.value(); + + Future _serialized(Future Function() op) { + final run = _lifecycle.then((_) => op()); + _lifecycle = run.then((_) {}, onError: (Object _) {}); + return run; + } + + Future start() => _serialized(_start); + + Future _start() async { if (_host != null) return; final version = await _version(); final host = McpSocketHost( @@ -125,7 +139,9 @@ class McpServerController { } /// Disconnects clients and removes the endpoint file. - Future stop() async { + Future stop() => _serialized(_stop); + + Future _stop() async { final host = _host; _host = null; for (final s in _servers.toList()) { @@ -166,11 +182,11 @@ class McpServerController { /// Stops and starts again with a new token; connected clients must /// reconnect (their `querya-mcp` reads the new endpoint file). - Future regenerateToken() async { - if (_host == null) return; - await stop(); - await start(); - } + Future regenerateToken() => _serialized(() async { + if (_host == null) return; + await _stop(); + await _start(); + }); void _publish(McpSocketHost host) { if (_host != host && _host != null) return; diff --git a/test/core/mcp/mcp_server_test.dart b/test/core/mcp/mcp_server_test.dart index 1d888234..ed41acb9 100644 --- a/test/core/mcp/mcp_server_test.dart +++ b/test/core/mcp/mcp_server_test.dart @@ -271,6 +271,47 @@ void main() { expect(msg['error']['message'], kMcpAppNotRunning); }); + test('overlapping start calls build one host, and stop closes it (#1563)', + () async { + final ports = {}; + void note() { + final port = controller.status.value.port; + if (port != null) ports.add(port); + } + + controller.status.addListener(note); + addTearDown(() => controller.status.removeListener(note)); + + await Future.wait([controller.start(), controller.start(), controller.start()]); + note(); + expect(ports, hasLength(1)); + + await controller.stop(); + for (final port in ports) { + await expectLater( + Socket.connect(InternetAddress.loopbackIPv4, port, + timeout: const Duration(milliseconds: 500)), + throwsA(isA()), + reason: 'no host may still listen on $port', + ); + } + }); + + test('stop waits for a start that is still running', () async { + final starting = controller.start(); + await controller.stop(); + await starting; + expect(controller.status.value.running, isFalse); + expect(await endpoint.exists(), isFalse); + }); + + test('a start after a stop works again', () async { + await controller.start(); + await controller.stop(); + await controller.start(); + expect(controller.status.value.running, isTrue); + }); + test('stopping the server disconnects a live client', () async { await controller.start(); final s = await _Session.open(endpoint);