Skip to content

webgl: fix intermittent headless test hangs#659

Open
mvdoc wants to merge 1 commit into
mainfrom
claude/webgl-headless-hang-fix
Open

webgl: fix intermittent headless test hangs#659
mvdoc wants to merge 1 commit into
mainfrom
claude/webgl-headless-hang-fix

Conversation

@mvdoc

@mvdoc mvdoc commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Supersedes #658 (that branch's ruleset blocked pushing the follow-up commit onto it, so this is a fresh branch with the complete fix in one commit).

The headless WebGL tests hung randomly and then failed in CI (e.g. run 29051424261: one RuntimeError: Failed to establish WebSocket connection ... within 60 seconds, then the whole job hit the 25-minute timeout-minutes guard ~15 min after pytest had already printed its summary). The job log also showed an unhandled thread exception OSError: [Errno 98] Address already in use in serve.py.

Three fixes, all in the headless viewer's connection setup:

1. Random-port collision with a silent bind failure (the visible test failure)

WebApp chose its port with random.randint(1024, 65536) and bound it asynchronously on the server thread via server.listen(port). When the random port was already taken, listen() raised OSError: [Errno 98] Address already in use on that thread and died silently, while server.start() had already returned. The caller then pointed headless Chromium at a port whose server never came up, so server.get_client() blocked for the full 60 s timeout and the test failed.

Fix: bind the listening socket synchronously in WebApp.__init__ via bind_sockets(0), letting the OS hand out a guaranteed-free ephemeral port (read back into self.port). Bind failures now raise in the caller, visibly, instead of dying on the server thread; the pre-bound socket also removes the connect race (the OS backlogs connections before the IOLoop starts). show() now requests port 0. As a side benefit, many simultaneous viewers are collision-free — each holds its own kernel-reserved ephemeral port for its lifetime.

2. Exit-time deadlock after the tests finished (the 25-minute job timeout)

headless_viewer awaited the WebSocket "connect" on a ThreadPoolExecutor worker running get_client(), which blocks on a no-timeout threading.Event. cancel_futures=True can't cancel an already-running future, so on a failed connection the worker parked forever — and concurrent.futures joins every worker at interpreter exit, wedging pytest until CI killed it.

Fix: await the client on a daemon thread (never joined at exit) with a bounded join, and set the connect event during teardown so the getter unblocks promptly.

3. start()/stop() race and socket leak in WebApp

With sockets now bound in __init__, stop() (which reads self.server / self.ioloop, set by run() on the server thread) could hit an AttributeError, be lost entirely, or leak the bound sockets if the server never started. (This is the point gemini-code-assist flagged on #658.)

Fix: a readiness Event set at the end of run()'s startup; stop() waits on it (bounded, short-circuited by is_alive()) before stopping the server + loop, and closes the bound sockets itself if the server never came up.

Testing

Exercised the real WebApp directly (standalone, since a full cortex import needs h5py/Chromium not present in the dev sandbox):

  • port=0 binds a real ephemeral port synchronously and the server serves HTTP on it.
  • An explicit in-use port raises OSError (errno 98) synchronously in the caller — no silent thread death.
  • 30 viewers constructed concurrently from threads got 30 distinct ports — zero collisions.
  • Every stop() path — normal serve→stop, never-started→stop (closes the socket, no leak, returns immediately), and 30× concurrent start+immediate-stop — is clean with no errors.

The predecessor #658 (fixes 1 & 2 only) already went green across the full run-tests matrix (Python 3.10–3.14) with no hang and no timeout; CI on this PR validates the complete change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PFExgXD5GN7kzoKZnnytkH


Generated by Claude Code

The headless WebGL tests hung randomly and then failed in CI (one
RuntimeError: "Failed to establish WebSocket connection ... within 60
seconds", then the whole job hit the 25-minute timeout guard ~15 min after
pytest had already printed its summary; the log also showed an unhandled
thread exception OSError: [Errno 98] Address already in use in serve.py).

Three fixes, all in the headless viewer's connection setup:

1. Random-port collision with a silent bind failure (the visible failure).
   WebApp picked its port with random.randint(1024, 65536) and bound it
   asynchronously via server.listen() on the server thread. When the random
   port was already in use, listen() raised "Address already in use" on that
   thread and died silently, while server.start() had already returned. The
   caller then navigated headless Chromium to a port whose server never came
   up, so server.get_client() blocked for the full 60s timeout.

   Fix: bind the listening socket synchronously in WebApp.__init__ with
   bind_sockets(0), letting the OS hand out a guaranteed-free ephemeral port
   (read back into self.port). Bind failures now raise in the caller instead
   of dying on the server thread, and the pre-bound socket removes the connect
   race (the OS backlogs connections before the IOLoop starts). show() now
   requests port 0. Many simultaneous viewers are also collision-free now,
   each holding its own kernel-reserved ephemeral port.

2. Exit-time deadlock after the tests finished (the 25-minute job timeout).
   headless_viewer awaited the WebSocket "connect" on a ThreadPoolExecutor
   worker running get_client(), which blocks on a no-timeout threading.Event.
   cancel_futures cannot cancel an already-running future, so on a failed
   connection the worker parked forever; concurrent.futures then joins every
   worker at interpreter exit, wedging pytest until CI killed it.

   Fix: await the client on a daemon thread (never joined at exit) with a
   bounded join, and set the connect event during teardown so the getter
   unblocks promptly.

3. start()/stop() race and socket leak in WebApp. With sockets now bound in
   __init__, stop() (which reads self.server / self.ioloop, set by run() on
   the server thread) could hit an AttributeError, be lost entirely, or leak
   the bound sockets if the server never started.

   Fix: a readiness Event set at the end of run()'s startup; stop() waits on
   it (bounded, short-circuited by is_alive()) before stopping the server and
   loop, and closes the bound sockets itself if the server never came up.

Verified against the real WebApp: ephemeral bind serves HTTP; an in-use port
raises OSError synchronously in the caller; 30 concurrent viewers get 30
distinct ports; and every stop() path (normal, never-started, concurrent
start+immediate-stop) is clean with no leak.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses intermittent headless and CI hangs by refactoring server startup and thread synchronization. Key changes include replacing ThreadPoolExecutor with a daemon thread to prevent shutdown hangs, binding sockets synchronously in the main thread using ephemeral ports (port=0) to avoid port collisions, and introducing a _ready event to safely coordinate server stops. The reviewer suggested using concurrent.futures.Future instead of a manual dictionary to simplify thread communication and exception propagation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread cortex/export/headless.py
Comment on lines +359 to +388
connect_result: dict[str, Any] = {}

def _await_client() -> None:
try:
connect_result["handle"] = server.get_client()
except BaseException as exc: # noqa: BLE001 - reported to main thread
connect_result["error"] = exc

client_thread = threading.Thread(
target=_await_client, name="headless-await-client", daemon=True
)

try:
# ------------------------------------------------------------------
# 3. Begin waiting for the WebSocket "connect" message in a thread
# *before* navigating, so we cannot miss it even if the browser
# connects before page.goto() returns.
# ------------------------------------------------------------------
pool = concurrent.futures.ThreadPoolExecutor(max_workers=1)
fut = pool.submit(server.get_client)
client_thread.start()
try:
# Launch the browser and navigate. python_interface.js runs on
# load and sends "connect" over WebSocket, which unblocks
# server.get_client().
pw_thread.start(url, timeout=timeout)

# Retrieve the handle; it should already be ready by this point,
# but the timeout guard surfaces hung state clearly.
handle = fut.result(timeout=timeout)
# Wait (bounded) for the getter to return; the timeout guard
# surfaces hung state clearly instead of blocking indefinitely.
client_thread.join(timeout=timeout)
if "error" in connect_result:
raise connect_result["error"]
if "handle" not in connect_result:
raise TimeoutError(
f"No WebSocket 'connect' received within {timeout:.0f}s"
)
handle = connect_result["handle"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a concurrent.futures.Future is a more idiomatic and robust way to pass results and exceptions between threads in Python. It simplifies the code by eliminating the manual dictionary-based state passing (connect_result), thread joining, and manual key/exception checks, while automatically propagating any exceptions or timeouts.

    connect_future: concurrent.futures.Future[Any] = concurrent.futures.Future()

    def _await_client() -> None:
        try:
            connect_future.set_result(server.get_client())
        except BaseException as exc:  # noqa: BLE001 - reported to main thread
            connect_future.set_exception(exc)

    client_thread = threading.Thread(
        target=_await_client, name="headless-await-client", daemon=True
    )

    try:
        client_thread.start()
        try:
            # Launch the browser and navigate.  python_interface.js runs on
            # load and sends "connect" over WebSocket, which unblocks
            # server.get_client().
            pw_thread.start(url, timeout=timeout)

            # Wait (bounded) for the getter to return; the timeout guard
            # surfaces hung state clearly instead of blocking indefinitely.
            handle = connect_future.result(timeout=timeout)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants