From ed17630b766385315999f82e9756a33dde8fdcd7 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Wed, 10 Jun 2026 23:23:56 -0700 Subject: [PATCH] =?UTF-8?q?fix(drone):=20broker=20start=5Fbackground=20blo?= =?UTF-8?q?cks=20until=20listening=20=E2=80=94=20kill=20connect-before-bin?= =?UTF-8?q?d=20race?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 9 +++++++++ src/aipass/drone/.seedgo/bypass.json | 2 +- src/aipass/drone/apps/handlers/broker/daemon.py | 9 +++++++-- src/aipass/drone/tests/test_broker.py | 5 ----- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f710f5f0..702c28f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,15 @@ and this project uses [Calendar Versioning](https://calver.org/) in the format (method-level, so the graceful no-broker paths still run on Windows). All skip on Windows and run unchanged on Linux. windows-setup was green pre-sandbox-merge (`00edd8b`) and red since (`0b4ba63`); this closes it. +- **Broker `start_background` connect-before-bind race.** `drone`'s out-of-sandbox + broker daemon started via `start_background()`, which returned *before* the + `AF_UNIX` socket was bound — callers then raced the bind, and on a slower machine + `create_identified_connection()` hit `FileNotFoundError` (socket not yet present). + Deterministic locally (`test_delete_nested_file` 0/5), green in CI only by timing + luck — latent flakiness. Fixed with a `threading.Event` set right after `listen()`; + `start_background(timeout=5.0)` now blocks on it and **raises** if the socket never + binds, so callers never guess a `sleep`. Removed the 4 blind `time.sleep(0.15)` + waits from the broker tests. Verified 55/55 broker tests, formerly-failing test 10/10. ### Added diff --git a/src/aipass/drone/.seedgo/bypass.json b/src/aipass/drone/.seedgo/bypass.json index f6559622..92c590d0 100644 --- a/src/aipass/drone/.seedgo/bypass.json +++ b/src/aipass/drone/.seedgo/bypass.json @@ -270,7 +270,7 @@ { "file": "apps/handlers/broker/daemon.py", "standard": "unused_function", - "lines": [420], + "lines": [422], "reason": "Threaded broker entrypoint, exercised by tests/test_broker.py; production uses blocking start(); intentionally not called in shipped non-test code." } ], diff --git a/src/aipass/drone/apps/handlers/broker/daemon.py b/src/aipass/drone/apps/handlers/broker/daemon.py index 1a9db1b4..4414422d 100644 --- a/src/aipass/drone/apps/handlers/broker/daemon.py +++ b/src/aipass/drone/apps/handlers/broker/daemon.py @@ -125,6 +125,7 @@ class BrokerDaemon: self._secret: bytes = b"" self._server: socket.socket | None = None self._running = False + self._listening = threading.Event() self._lock = threading.Lock() json_handler.log_operation( "broker_init", @@ -400,6 +401,7 @@ class BrokerDaemon: self._server.listen(5) self._server.settimeout(1.0) self._running = True + self._listening.set() logger.info("broker: listening on %s", self.socket_path) json_handler.log_operation("broker_start", {"socket": str(self.socket_path)}) @@ -417,10 +419,13 @@ class BrokerDaemon: logger.error("broker: accept error: %s", exc) break - def start_background(self) -> threading.Thread: - """Start the broker in a background thread. Returns the thread.""" + def start_background(self, timeout: float = 5.0) -> threading.Thread: + """Start the broker in a background thread, blocking until listening.""" + self._listening.clear() t = threading.Thread(target=self.start, daemon=True, name="drone-broker") t.start() + if not self._listening.wait(timeout): + raise RuntimeError(f"broker failed to start listening within {timeout}s") return t def stop(self) -> None: diff --git a/src/aipass/drone/tests/test_broker.py b/src/aipass/drone/tests/test_broker.py index b9ea145d..5692da41 100644 --- a/src/aipass/drone/tests/test_broker.py +++ b/src/aipass/drone/tests/test_broker.py @@ -23,7 +23,6 @@ import json import socket import os import stat -import time from pathlib import Path import pytest @@ -125,7 +124,6 @@ def broker(tmp_path: Path, repo_root: Path) -> BrokerDaemon: def running_broker(broker: BrokerDaemon): """Start a broker in background, yield it, stop on teardown.""" t = broker.start_background() - time.sleep(0.15) yield broker broker.stop() t.join(timeout=3) @@ -367,7 +365,6 @@ class TestBrokerDaemon: def test_stop_cleans_socket(self, broker: BrokerDaemon) -> None: """Stopping the broker removes the socket file.""" t = broker.start_background() - time.sleep(0.15) assert broker.socket_path.exists() broker.stop() t.join(timeout=3) @@ -574,7 +571,6 @@ class TestIdentity: secret_path=secret_path, ) t1 = d1.start_background() - time.sleep(0.15) secret1 = secret_path.read_bytes() d1.stop() t1.join(timeout=3) @@ -586,7 +582,6 @@ class TestIdentity: secret_path=secret_path, ) t2 = d2.start_background() - time.sleep(0.15) secret2 = secret_path.read_bytes() d2.stop() t2.join(timeout=3)