Skip to content

Commit 1bb280e

Browse files
committed
Revert "Server: refine shutdown logic per bot feedback\n- Parameterize _force_exit(code) and use timers with args\n- Consistent behavior on BrokenPipeError (no immediate exit)\n- Exit code 1 on unexpected exceptions\n\nTests: restore telemetry module after disabling to avoid bleed-over"
This reverts commit 74d35d3.
1 parent 74d35d3 commit 1bb280e

File tree

2 files changed

+27
-110
lines changed

2 files changed

+27
-110
lines changed

Server/server.py

Lines changed: 1 addition & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44
from logging.handlers import RotatingFileHandler
55
import os
66
from contextlib import asynccontextmanager
7-
import sys
8-
import signal
9-
import threading
107
from typing import AsyncIterator, Dict, Any
118
from config import config
129
from tools import register_all_tools
@@ -67,9 +64,6 @@
6764
# Global connection state
6865
_unity_connection: UnityConnection = None
6966

70-
# Global shutdown coordination
71-
_shutdown_flag = threading.Event()
72-
7367

7468
@asynccontextmanager
7569
async def server_lifespan(server: FastMCP) -> AsyncIterator[Dict[str, Any]]:
@@ -192,96 +186,9 @@ def _emit_startup():
192186
register_all_resources(mcp)
193187

194188

195-
def _force_exit(code: int = 0):
196-
"""Force process exit, bypassing any background threads that might linger."""
197-
try:
198-
sys.exit(code)
199-
except SystemExit:
200-
os._exit(code)
201-
202-
203-
def _signal_handler(signum, frame):
204-
logger.info(f"Received signal {signum}, initiating shutdown...")
205-
_shutdown_flag.set()
206-
threading.Timer(1.0, _force_exit, args=(0,)).start()
207-
208-
209-
def _monitor_stdin():
210-
"""Background thread to detect stdio detach (stdin EOF) or parent exit."""
211-
try:
212-
parent_pid = os.getppid() if hasattr(os, "getppid") else None
213-
while not _shutdown_flag.is_set():
214-
if _shutdown_flag.wait(0.5):
215-
break
216-
217-
if parent_pid is not None:
218-
try:
219-
os.kill(parent_pid, 0)
220-
except (ProcessLookupError, OSError):
221-
logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
222-
break
223-
224-
try:
225-
if sys.stdin.closed:
226-
logger.info("stdin.closed is True; client disconnected")
227-
break
228-
fd = sys.stdin.fileno()
229-
if fd < 0:
230-
logger.info("stdin fd invalid; client disconnected")
231-
break
232-
except (ValueError, OSError, AttributeError):
233-
# Closed pipe or unavailable stdin
234-
break
235-
except Exception:
236-
# Ignore transient errors
237-
pass
238-
239-
if not _shutdown_flag.is_set():
240-
logger.info("Client disconnected (stdin or parent), initiating shutdown...")
241-
_shutdown_flag.set()
242-
if not _shutdown_flag.is_set():
243-
threading.Timer(0.5, _force_exit, args=(0,)).start()
244-
else:
245-
threading.Timer(0.5, _force_exit, args=(0,)).start()
246-
except Exception:
247-
# Never let monitor thread crash the process
248-
pass
249-
250-
251189
def main():
252190
"""Entry point for uvx and console scripts."""
253-
try:
254-
signal.signal(signal.SIGTERM, _signal_handler)
255-
signal.signal(signal.SIGINT, _signal_handler)
256-
if hasattr(signal, "SIGPIPE"):
257-
signal.signal(signal.SIGPIPE, signal.SIG_IGN)
258-
if hasattr(signal, "SIGBREAK"):
259-
signal.signal(signal.SIGBREAK, _signal_handler)
260-
except Exception:
261-
# Signals can fail in some environments
262-
pass
263-
264-
t = threading.Thread(target=_monitor_stdin, daemon=True)
265-
t.start()
266-
267-
try:
268-
mcp.run(transport='stdio')
269-
logger.info("FastMCP run() returned (stdin EOF or disconnect)")
270-
except (KeyboardInterrupt, SystemExit):
271-
logger.info("Server interrupted; shutting down")
272-
_shutdown_flag.set()
273-
except BrokenPipeError:
274-
logger.info("Broken pipe; shutting down")
275-
_shutdown_flag.set()
276-
# rely on finally to schedule exit for consistency
277-
except Exception as e:
278-
logger.error(f"Server error: {e}", exc_info=True)
279-
_shutdown_flag.set()
280-
_force_exit(1)
281-
finally:
282-
_shutdown_flag.set()
283-
logger.info("Server main loop exited")
284-
threading.Timer(0.5, _force_exit, args=(0,)).start()
191+
mcp.run(transport='stdio')
285192

286193

287194
# Run the server

Server/test_telemetry.py

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ def test_telemetry_basic():
2323
)
2424
pass
2525
except ImportError as e:
26-
# Fail explicitly when imports are missing
27-
assert False, f"telemetry import failed: {e}"
26+
# Silent failure path for tests
27+
return False
2828

2929
# Test telemetry enabled status
3030
_ = is_telemetry_enabled()
@@ -37,7 +37,8 @@ def test_telemetry_basic():
3737
})
3838
pass
3939
except Exception as e:
40-
assert False, f"record_telemetry failed: {e}"
40+
# Silent failure path for tests
41+
return False
4142

4243
# Test milestone recording
4344
try:
@@ -46,23 +47,26 @@ def test_telemetry_basic():
4647
})
4748
_ = is_first
4849
except Exception as e:
49-
assert False, f"record_milestone failed: {e}"
50+
# Silent failure path for tests
51+
return False
5052

5153
# Test telemetry collector
5254
try:
5355
collector = get_telemetry()
5456
_ = collector
5557
except Exception as e:
56-
assert False, f"get_telemetry failed: {e}"
57-
assert True
58+
# Silent failure path for tests
59+
return False
5860

61+
return True
5962

60-
def test_telemetry_disabled(monkeypatch):
63+
64+
def test_telemetry_disabled():
6165
"""Test telemetry with disabled state"""
6266
# Silent for tests
6367

6468
# Set environment variable to disable telemetry
65-
monkeypatch.setenv("DISABLE_TELEMETRY", "true")
69+
os.environ["DISABLE_TELEMETRY"] = "true"
6670

6771
# Re-import to get fresh config
6872
import importlib
@@ -73,12 +77,17 @@ def test_telemetry_disabled(monkeypatch):
7377

7478
_ = is_telemetry_enabled()
7579

76-
assert is_telemetry_enabled() is False
77-
# Test that records are ignored when disabled (should not raise)
78-
record_telemetry(RecordType.USAGE, {"test": "should_be_ignored"})
79-
# Restore module state for subsequent tests
80-
monkeypatch.delenv("DISABLE_TELEMETRY", raising=False)
81-
importlib.reload(telemetry)
80+
if not is_telemetry_enabled():
81+
pass
82+
83+
# Test that records are ignored when disabled
84+
record_telemetry(RecordType.USAGE, {"test": "should_be_ignored"})
85+
pass
86+
87+
return True
88+
else:
89+
pass
90+
return False
8291

8392

8493
def test_data_storage():
@@ -105,10 +114,11 @@ def test_data_storage():
105114
else:
106115
pass
107116

108-
assert True
117+
return True
109118

110119
except Exception as e:
111-
assert False, f"data storage test failed: {e}"
120+
# Silent failure path for tests
121+
return False
112122

113123

114124
def main():

0 commit comments

Comments
 (0)