Visitar URL original
Send pub/sub messages on the server's event loop by CraigBurden · Pull Request #1349 · google/openhtf · GitHub
Skip to content

Send pub/sub messages on the server's event loop - #1349

Open
CraigBurden wants to merge 1 commit into
google:masterfrom
CraigBurden:fix-pubsub-event-loop
Open

CraigBurden wants to merge 1 commit into
google:masterfrom
CraigBurden:fix-pubsub-event-loop

Conversation

@CraigBurden

Copy link
Copy Markdown

Send pub/sub messages on the server's event loop

Problem

PubSub.publish() is called from the StationWatcher thread and from test output callbacks (StationServer.publish_final_state), not from the tornado event loop that owns the websocket connections. sockjs-tornado's websocket transport calls write_message() directly, so off-loop:

  • the frame is written to the IOStream from the wrong thread, which isn't thread-safe, and
  • write_message() schedules its completion task on the calling thread's event loop. StationWatcher creates that loop but never runs it, so every task stays pending for the life of the process.

At shutdown every one of these tasks gets reported:

Task was destroyed but it is pending!
task: <Task pending name='Task-8' coro=<WebSocketProtocol13.write_message.<locals>.wrapper() running at .../tornado/websocket.py:1111>>

A station that runs for a shift with the GUI open logs thousands of these lines on exit.

Fix

PubSub records the serving loop when the first client subscribes (on_open always runs on that loop). publish() sends directly when it's already on that loop, and otherwise hands the send to the loop with call_soon_threadsafe. If nobody has subscribed, it does nothing, as before. If the loop has closed, the publish is dropped, because the server is shutting down. _loop is stored per subclass, like _lock and subscribers, so StationPubSub and DashboardPubSub don't share it.

Testing

  • New test/output/servers/pub_sub_test.py: publishing from another thread, from the serving loop, with a client filter, with no subscribers, and after the loop has closed. Three of its cases fail without the fix.

  • Full suite passes: pytest test (588 passed).

  • I also ran a script that starts a StationServer, connects a websocket client and runs 5 tests that each log heavily and then prompt. The client answers each prompt over HTTP.

    "Task was destroyed" at exit Prompt latency (median) Plug POSTs
    master 8–10 0.22–0.39 s 5 × 200
    this PR 0 0.20–0.27 s 5 × 200

🤖 Generated with Claude Code

PubSub.publish() is called from the StationWatcher thread and from test
output callbacks, not the tornado loop that owns the websocket
connections. Called off-loop, write_message() still writes the frame
(unsafely, from the wrong thread), but schedules its completion task on
the calling thread's event loop, which never runs. Those tasks leak for
the life of the process and are destroyed at exit, logging
"Task was destroyed but it is pending!" for every message published.

Capture the serving loop when the first client subscribes and hand
off-loop publishes to it with call_soon_threadsafe.

This branch has not been deployed

No deployments
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.

1 participant