Skip to content

Commit b98ca7f

Browse files
committed
fix(web): serve on a background thread so Ctrl-C stops --serve/--web on Windows
Winsock select() inside serve_forever() ignores the SIGINT event, so a foreground serve loop never sees Ctrl-C on Windows and --serve/--web were unkillable from the keyboard. Run serve_forever on a daemon thread and block the main thread on an interruptible Thread.join(), then tear down via shutdown()/server_close(). Requests still run one at a time, preserving the stores' no-concurrent-access invariant.
1 parent 3b196e3 commit b98ca7f

2 files changed

Lines changed: 66 additions & 7 deletions

File tree

src/opentab/web.py

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -341,10 +341,13 @@ class ReportServer(HTTPServer):
341341
(payload included) is built once and cached; /api/reload re-reads the stores
342342
and invalidates it -- wired to the page's refresh button.
343343
344-
Deliberately single-threaded: the stores' sqlite connections are bound to the
345-
thread that created them (check_same_thread), and every request is fast (the
346-
page is cached, the extras are the same cheap per-session queries the TUI runs
347-
on drill-in), so serializing a single local user's requests costs nothing."""
344+
Deliberately single-threaded: the stores forbid concurrent access to a
345+
connection, and every request is fast (the page is cached, the extras are the
346+
same cheap per-session queries the TUI runs on drill-in), so serializing a
347+
single local user's requests on one serve thread costs nothing. serve_command
348+
runs serve_forever on a background thread (all requests still one at a time on
349+
it) so the main thread can catch a Ctrl-C that Windows won't deliver to
350+
serve_forever's select()."""
348351

349352
def __init__(self, address: tuple[str, int], app: App):
350353
super().__init__(address, _Handler)
@@ -393,19 +396,33 @@ def serve_command(app: App, args: argparse.Namespace) -> int:
393396
host = "localhost" if bind in ("127.0.0.1", "::1") else bind
394397
url = f"http://{host}:{server.server_address[1]}/"
395398
print(f"opentab report at {url} (Ctrl-C to stop)")
399+
import threading
400+
396401
if getattr(args, "web", False):
397402
# --web: pop the browser now the socket is listening (bound in __init__, so a
398403
# request racing serve_forever just queues in the backlog). In a daemon thread
399404
# so a console-browser fallback that runs in the foreground can't block
400405
# serve_forever; it only calls webbrowser, never the sqlite-bound store, so
401406
# the single-threaded-request design still holds.
402-
import threading
403-
404407
threading.Thread(target=open_report, args=(url,), daemon=True).start()
408+
# Serve on a background thread and block the main thread on an interruptible join --
409+
# NOT serve_forever() on the main thread. On Windows a Ctrl-C never wakes the
410+
# select() inside serve_forever (Winsock select ignores the SIGINT event, so the
411+
# KeyboardInterrupt stays pending until some request happens to return control to the
412+
# eval loop), which left --serve/--web unkillable from the keyboard. Thread.join()
413+
# instead waits on a lock whose acquire IS wired to the SIGINT event on Windows, so
414+
# Ctrl-C interrupts it at once. Requests still run one at a time on the single serve
415+
# thread, so the store's no-concurrent-access invariant holds; daemon=True is a
416+
# backstop so the process can still exit even mid-request.
417+
server_thread = threading.Thread(target=server.serve_forever, name="opentab-serve", daemon=True)
418+
server_thread.start()
405419
try:
406-
server.serve_forever()
420+
while server_thread.is_alive():
421+
server_thread.join(0.5) # interruptible by Ctrl-C on Windows, unlike select()
407422
except KeyboardInterrupt:
408423
pass
409424
finally:
425+
server.shutdown() # off the serve thread, so it stops the loop without deadlock
410426
server.server_close()
427+
server_thread.join(timeout=2)
411428
return 0

test_opentab.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8717,6 +8717,48 @@ def test_web_server_is_hardened_against_csrf_and_dns_rebinding():
87178717
thread.join(timeout=5)
87188718

87198719

8720+
def test_serve_command_runs_serve_forever_off_the_main_thread():
8721+
# Regression: on Windows a Ctrl-C never wakes serve_forever's select(), so serving
8722+
# in the foreground was unkillable from the keyboard. serve_command must run
8723+
# serve_forever on a background thread (leaving the main thread free to catch the
8724+
# interrupt) and always tear the server down via shutdown() + server_close().
8725+
import threading
8726+
import types
8727+
8728+
class FakeServer:
8729+
def __init__(self, address, app):
8730+
self.server_address = (address[0], 8765)
8731+
self.events = []
8732+
self.serve_on_main = None
8733+
8734+
def page(self):
8735+
self.events.append("page")
8736+
8737+
def serve_forever(self):
8738+
self.serve_on_main = threading.current_thread() is threading.main_thread()
8739+
self.events.append("serve") # returns at once -> the join loop then exits
8740+
8741+
def shutdown(self):
8742+
self.events.append("shutdown")
8743+
8744+
def server_close(self):
8745+
self.events.append("close")
8746+
8747+
made = {}
8748+
real = ot.web.ReportServer
8749+
ot.web.ReportServer = lambda address, app: made.setdefault("s", FakeServer(address, app))
8750+
try:
8751+
args = types.SimpleNamespace(bind="127.0.0.1", port=0, web=False)
8752+
rc = ot.web.serve_command(app_with([workflow("w1", "2026-05-01 10:00:00")]), args)
8753+
finally:
8754+
ot.web.ReportServer = real
8755+
server = made["s"]
8756+
assert rc == 0
8757+
assert server.serve_on_main is False # never the foreground / main thread
8758+
assert "serve" in server.events
8759+
assert server.events[-2:] == ["shutdown", "close"] # always torn down
8760+
8761+
87208762
def test_cli_web_flag_is_recognized_and_is_distinct_from_serve():
87218763
# --web is its own flag; web_command/main route it through the serve path.
87228764
import sys as _sys

0 commit comments

Comments
 (0)