fix(drone): broker start_background blocks until listening — kill connect-before-bind race
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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."
|
||||
}
|
||||
],
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user