httpserver: Fix race condition in server startup

pytest_httpserver/blocking_httpserver.py

@@ -46,6 +46,8 @@ class BlockingHTTPServer(HTTPServerBase):
 
     :param timeout: waiting time in seconds for matching and responding to an incoming request.
         manager
+    :param startup_timeout: maximum time in seconds to wait for server readiness.
+        Set to ``None`` to disable readiness waiting.
 
     .. py:attribute:: no_handler_status_code
 
@@ -63,8 +65,10 @@ def __init__(
         port: int = DEFAULT_LISTEN_PORT,
         ssl_context: SSLContext | None = None,
         timeout: int = 30,
+        *,
+        startup_timeout: float | None = 10.0,
     ) -> None:
-        super().__init__(host, port, ssl_context)
+        super().__init__(host, port, ssl_context, startup_timeout=startup_timeout)
         self.timeout = timeout
         self.request_queue: Queue[Request] = Queue()
         self.request_handlers: dict[Request, Queue[BlockingRequestHandler]] = {}

pytest_httpserver/httpserver.py

@@ -8,6 +8,7 @@
 import threading
 import time
 import urllib.parse
+import warnings
 from collections import defaultdict
 from collections.abc import Callable
 from collections.abc import Generator
@@ -615,6 +616,8 @@ class HTTPServerBase(abc.ABC):  # pylint: disable=too-many-instance-attributes
     :param port: the TCP port where the server will listen
     :param ssl_context: the ssl context object to use for https connections
     :param threaded: whether to handle concurrent requests in separate threads
+    :param startup_timeout: maximum time in seconds to wait for server readiness.
+        Set to ``None`` to disable readiness waiting.
 
     .. py:attribute:: log
 
@@ -638,6 +641,7 @@ def __init__(
         ssl_context: SSLContext | None = None,
         *,
         threaded: bool = False,
+        startup_timeout: float | None = 10.0,
     ) -> None:
         """
         Initializes the instance.
@@ -652,7 +656,9 @@ def __init__(
         self.log: list[tuple[Request, Response]] = []
         self.ssl_context = ssl_context
         self.threaded = threaded
+        self.startup_timeout = startup_timeout
         self.no_handler_status_code = 500
+        self._server_ready_event: threading.Event = threading.Event()
 
     def __repr__(self) -> str:
         return f"<{self.__class__.__name__} host={self.host} port={self.port}>"
@@ -733,8 +739,14 @@ def thread_target(self) -> None:
         This method serves as a thread target when the server is started.
 
         This should not be called directly, but can be overridden to tailor it to your needs.
+
+        If overriding, you should call ``self._server_ready_event.set()`` before starting
+        to serve requests. If the event is not set within the timeout, :py:meth:`start`
+        will emit a warning if the thread is still alive; if the thread dies during
+        startup, :py:meth:`start` raises an error.
         """
         assert self.server is not None
+        self._server_ready_event.set()
         self.server.serve_forever()
 
     def is_running(self) -> bool:
@@ -774,8 +786,33 @@ def start(self) -> None:
 
         self.port = self.server.port  # Update port (needed if `port` was set to 0)
         # Explicitly make the new thread daemonic to avoid shutdown issues
+        # Create a new event for each startup to prevent stale threads from
+        # signaling readiness for a subsequent start() attempt.
+        self._server_ready_event = threading.Event()
         self.server_thread = threading.Thread(target=self.thread_target, daemon=True)
         self.server_thread.start()
+        if self.startup_timeout is not None and not self._server_ready_event.wait(timeout=self.startup_timeout):
+            # Event was not set within timeout.
+            # Check if thread is still alive (custom thread_target may not set the event)
+            if self.server_thread.is_alive():
+                # Server thread is running, assume it's working (backward compatibility)
+                warnings.warn(
+                    "Server thread is running but ready event was not set. "
+                    "If you override thread_target(), call self._server_ready_event.set() "
+                    "before serving to ensure reliable startup.",
+                    stacklevel=2,
+                )
+            else:
+                # Thread died, clean up and raise
+                self.server.server_close()
+                self.server_thread.join(timeout=5)
+                self.server = None
+                self.server_thread = None
+                raise HTTPServerError(
+                    "Server thread died during startup. "
+                    "If you override thread_target(), ensure it calls "
+                    "self._server_ready_event.set() before serving."
+                )
 
     def stop(self) -> None:
         """
@@ -937,6 +974,8 @@ class HTTPServer(HTTPServerBase):  # pylint: disable=too-many-instance-attribute
         manager
 
     :param threaded: whether to handle concurrent requests in separate threads
+    :param startup_timeout: maximum time in seconds to wait for server readiness.
+        Set to ``None`` to disable readiness waiting.
 
     .. py:attribute:: no_handler_status_code
 
@@ -956,11 +995,18 @@ def __init__(
         default_waiting_settings: WaitingSettings | None = None,
         *,
         threaded: bool = False,
+        startup_timeout: float | None = 10.0,
     ) -> None:
         """
         Initializes the instance.
         """
-        super().__init__(host, port, ssl_context, threaded=threaded)
+        super().__init__(
+            host,
+            port,
+            ssl_context,
+            threaded=threaded,
+            startup_timeout=startup_timeout,
+        )
 
         self.ordered_handlers: list[RequestHandler] = []
         self.oneshot_handlers = RequestHandlerList()

tests/test_release.py

@@ -232,6 +232,7 @@ def test_sdist_contents(build: Build, version: str):
             "test_querymatcher.py",
             "test_querystring.py",
             "test_release.py",
+            "test_server_startup.py",
             "test_ssl.py",
             "test_thread_type.py",
             "test_threaded.py",

tests/test_server_startup.py

@@ -0,0 +1,188 @@
+from __future__ import annotations
+
+import contextlib
+import socket
+import time
+
+import pytest
+import requests
+from requests.exceptions import Timeout
+
+from pytest_httpserver import HTTPServer
+
+
+def test_server_ready_immediately_after_start() -> None:
+    """Test that the server accepts connections immediately after start() returns."""
+    server = HTTPServer(host="localhost", port=0)
+    server.expect_request("/").respond_with_data("ok")
+    server.start()
+    try:
+        # Attempt to connect immediately - should not fail
+        sock = socket.create_connection((server.host, server.port), timeout=1)
+        sock.close()
+    finally:
+        server.stop()
+
+
+def test_server_ready_under_load() -> None:
+    """Test that the server is ready even when started multiple times in succession."""
+    for _ in range(10):
+        server = HTTPServer(host="localhost", port=0)
+        server.expect_request("/").respond_with_data("ok")
+        server.start()
+        try:
+            sock = socket.create_connection((server.host, server.port), timeout=1)
+            sock.close()
+        finally:
+            server.stop()
+
+
+class SlowStartServer(HTTPServer):
+    """A server subclass that simulates slow startup."""
+
+    def thread_target(self) -> None:
+        time.sleep(0.5)  # Simulate slow initialization
+        self._server_ready_event.set()
+        assert self.server is not None
+        self.server.serve_forever()
+
+
+class NoReadyEventServer(HTTPServer):
+    """A server subclass that never signals readiness."""
+
+    def thread_target(self) -> None:
+        assert self.server is not None
+        self.server.serve_forever()
+
+
+def test_slow_start_server_waits_for_ready() -> None:
+    """Test that start() waits for slow thread_target implementations."""
+    server = SlowStartServer(host="localhost", port=0)
+    server.expect_request("/").respond_with_data("ok")
+
+    start_time = time.monotonic()
+    server.start()
+    elapsed = time.monotonic() - start_time
+
+    try:
+        # Should have waited at least 0.5 seconds
+        assert elapsed >= 0.5
+        # Server should be ready
+        sock = socket.create_connection((server.host, server.port), timeout=1)
+        sock.close()
+    finally:
+        server.stop()
+
+
+def test_new_event_created_for_each_start() -> None:
+    """Test that a new event is created for each start() to isolate retries."""
+    server = HTTPServer(host="localhost", port=0)
+    server.expect_request("/").respond_with_data("ok")
+
+    original_event = server._server_ready_event  # noqa: SLF001
+
+    server.start()
+    first_start_event = server._server_ready_event  # noqa: SLF001
+    server.stop()
+
+    server.start()
+    second_start_event = server._server_ready_event  # noqa: SLF001
+    server.stop()
+
+    # Each start() should create a new event
+    assert first_start_event is not original_event
+    assert second_start_event is not first_start_event
+
+
+def test_warns_when_ready_event_not_set() -> None:
+    """Test that a warning is emitted when the ready event is never set."""
+    server = NoReadyEventServer(host="localhost", port=0, startup_timeout=0.0)
+    server.expect_request("/").respond_with_data("ok")
+
+    with pytest.warns(UserWarning, match="ready event was not set"):
+        server.start()
+
+    try:
+        deadline = time.time() + 1
+        while time.time() < deadline:
+            with contextlib.suppress(OSError):
+                sock = socket.create_connection((server.host, server.port), timeout=0.1)
+                sock.close()
+                break
+            time.sleep(0.01)
+        else:
+            raise AssertionError("Server did not accept connections within 1 second")
+    finally:
+        server.stop()
+
+
+class SlowServeServer(HTTPServer):
+    """A server that delays serve_forever() but does not set ready event early.
+
+    This simulates the scenario where:
+    - bind() and listen() complete (TCP connections queue in backlog)
+    - But serve_forever() hasn't started yet (no HTTP responses)
+    """
+
+    def thread_target(self) -> None:
+        assert self.server is not None
+        # Delay before serve_forever - connections will queue but not be processed
+        time.sleep(3.0)
+        self._server_ready_event.set()
+        self.server.serve_forever()
+
+
+def test_http_request_fails_before_serve_forever_without_wait() -> None:
+    """
+    Demonstrate the race condition: TCP connects but HTTP times out.
+
+    This test shows why waiting for server readiness matters:
+    - After start(), TCP connections succeed (queued in backlog)
+    - But HTTP requests timeout because serve_forever() hasn't started
+    - With short client timeouts (common in production), this causes failures
+    """
+    # Use startup_timeout=0 to NOT wait for ready event (old behavior)
+    server = SlowServeServer(host="localhost", port=0, startup_timeout=0.0)
+    server.expect_request("/ping").respond_with_data("pong")
+
+    with pytest.warns(UserWarning, match="ready event was not set"):
+        server.start()
+
+    try:
+        # TCP connection succeeds (proves Zsolt's point about backlog)
+        sock = socket.create_connection((server.host, server.port), timeout=1)
+        sock.close()
+
+        # But HTTP request with short timeout fails!
+        # This is the actual problem in containerized environments
+        with pytest.raises(Timeout):
+            requests.get(server.url_for("/ping"), timeout=(0.5, 0.5))
+    finally:
+        server.stop()
+
+
+def test_http_request_succeeds_when_waiting_for_ready() -> None:
+    """
+    Demonstrate that waiting for ready event fixes the race condition.
+
+    With startup_timeout enabled (default), start() waits until
+    serve_forever() begins, so HTTP requests succeed immediately.
+    """
+    # Use default startup_timeout to wait for ready event
+    server = SlowServeServer(host="localhost", port=0)  # default startup_timeout=10.0
+    server.expect_request("/ping").respond_with_data("pong")
+
+    start_time = time.monotonic()
+    server.start()
+    elapsed = time.monotonic() - start_time
+
+    try:
+        # Should have waited for the slow startup
+        assert elapsed >= 3.0, f"Expected to wait >= 3.0s, but only waited {elapsed}s"
+
+        # HTTP request succeeds because serve_forever() has started
+        response = requests.get(server.url_for("/ping"), timeout=(0.5, 0.5))
+        assert response.status_code == 200
+        assert response.text == "pong"
+    finally:
+        server.stop()