fix: remove browser_tool signal handlers that cause voice mode deadlock
browser_tool.py registered SIGINT/SIGTERM handlers that called sys.exit() at module import time. When a signal arrived during a lock acquisition (e.g. AudioRecorder._lock in voice mode), SystemExit was raised inside prompt_toolkit's async event loop, corrupting coroutine state and making the process unkillable (required SIGKILL). atexit handler already ensures browser sessions are cleaned up on any normal exit path, so the signal handlers were redundant and harmful.
This commit is contained in:
@@ -674,3 +674,30 @@ class TestChatTTSCleanupOnException:
|
||||
"chat() must have a finally block cleaning up "
|
||||
"text_queue/stop_event/tts_thread"
|
||||
)
|
||||
|
||||
|
||||
class TestBrowserToolSignalHandlerRemoved:
|
||||
"""browser_tool.py must NOT register SIGINT/SIGTERM handlers that call
|
||||
sys.exit() — this conflicts with prompt_toolkit's event loop and causes
|
||||
the process to become unkillable during voice mode."""
|
||||
|
||||
def test_no_signal_handler_registration(self):
|
||||
"""Source check: browser_tool.py must not call signal.signal()
|
||||
for SIGINT or SIGTERM."""
|
||||
with open("tools/browser_tool.py") as f:
|
||||
source = f.read()
|
||||
|
||||
lines = source.split("\n")
|
||||
for i, line in enumerate(lines, 1):
|
||||
stripped = line.strip()
|
||||
# Skip comments
|
||||
if stripped.startswith("#"):
|
||||
continue
|
||||
assert "signal.signal(signal.SIGINT" not in stripped, (
|
||||
f"browser_tool.py:{i} registers SIGINT handler — "
|
||||
f"use atexit instead to avoid prompt_toolkit conflicts"
|
||||
)
|
||||
assert "signal.signal(signal.SIGTERM" not in stripped, (
|
||||
f"browser_tool.py:{i} registers SIGTERM handler — "
|
||||
f"use atexit instead to avoid prompt_toolkit conflicts"
|
||||
)
|
||||
|
||||
@@ -224,24 +224,14 @@ def _emergency_cleanup_all_sessions():
|
||||
logger.error("Emergency cleanup error: %s", e)
|
||||
|
||||
|
||||
def _signal_handler(signum, frame):
|
||||
"""Handle interrupt signals to cleanup sessions before exit."""
|
||||
logger.warning("Received signal %s, cleaning up...", signum)
|
||||
_emergency_cleanup_all_sessions()
|
||||
sys.exit(128 + signum)
|
||||
|
||||
|
||||
# Register cleanup handlers
|
||||
# Register cleanup via atexit only. Previous versions installed SIGINT/SIGTERM
|
||||
# handlers that called sys.exit(), but this conflicts with prompt_toolkit's
|
||||
# async event loop — a SystemExit raised inside a key-binding callback
|
||||
# corrupts the coroutine state and makes the process unkillable. atexit
|
||||
# handlers run on any normal exit (including sys.exit), so browser sessions
|
||||
# are still cleaned up without hijacking signals.
|
||||
atexit.register(_emergency_cleanup_all_sessions)
|
||||
|
||||
# Only register signal handlers in main process (not in multiprocessing workers)
|
||||
try:
|
||||
if os.getpid() == os.getpgrp(): # Main process check
|
||||
signal.signal(signal.SIGINT, _signal_handler)
|
||||
signal.signal(signal.SIGTERM, _signal_handler)
|
||||
except (OSError, AttributeError):
|
||||
pass # Signal handling not available (e.g., Windows or worker process)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Inactivity Cleanup Functions
|
||||
|
||||
Reference in New Issue
Block a user