Skip to content

Server: Robust shutdown on stdio detach (signals, stdin/parent monitor, forced exit) #363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 1, 2025
+357 −164
Merged

Server: Robust shutdown on stdio detach (signals, stdin/parent monitor, forced exit) #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2b5f77e
Server: robust shutdown on stdio detach (signals, stdin/parent monito…
dsarno Oct 31, 2025
e586145
Server: simplify _force_exit to os._exit; guard exit timers to avoid …
dsarno Oct 31, 2025
dfd1238
Server: add DEBUG logs for transient stdin checks and monitor thread …
dsarno Oct 31, 2025
32b35c9
Mirror shutdown improvements: signal handlers, stdin/parent monitor, …
dsarno Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 97 additions & 1 deletion MCPForUnity/UnityMcpServer~/src/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
from logging.handlers import RotatingFileHandler
import os
from contextlib import asynccontextmanager
import signal
import sys
import threading
from typing import AsyncIterator, Dict, Any
from config import config
from tools import register_all_tools
Expand Down Expand Up @@ -64,6 +67,10 @@
# Global connection state
_unity_connection: UnityConnection = None

# Global shutdown coordination
_shutdown_flag = threading.Event()
_exit_timer_scheduled = threading.Event()


@asynccontextmanager
async def server_lifespan(server: FastMCP) -> AsyncIterator[Dict[str, Any]]:
Expand Down Expand Up @@ -186,9 +193,98 @@ def _emit_startup():
register_all_resources(mcp)


def _force_exit(code: int = 0):
"""Force process exit, bypassing any background threads that might linger."""
os._exit(code)


def _signal_handler(signum, frame):
logger.info(f"Received signal {signum}, initiating shutdown...")
_shutdown_flag.set()
if not _exit_timer_scheduled.is_set():
_exit_timer_scheduled.set()
threading.Timer(1.0, _force_exit, args=(0,)).start()


def _monitor_stdin():
"""Background thread to detect stdio detach (stdin EOF) or parent exit."""
try:
parent_pid = os.getppid() if hasattr(os, "getppid") else None
while not _shutdown_flag.is_set():
if _shutdown_flag.wait(0.5):
break

if parent_pid is not None:
try:
os.kill(parent_pid, 0)
except ValueError:
# Signal 0 unsupported on this platform (e.g., Windows); disable parent probing
parent_pid = None
except (ProcessLookupError, OSError):
logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
break

try:
if sys.stdin.closed:
logger.info("stdin.closed is True; client disconnected")
break
fd = sys.stdin.fileno()
if fd < 0:
logger.info("stdin fd invalid; client disconnected")
break
except (ValueError, OSError, AttributeError):
# Closed pipe or unavailable stdin
Comment on lines +217 to +236
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Prevent false parent-death detection on Windows.

Identical to the bridge variant: os.kill(parent_pid, 0) throws PermissionError/EINVAL on Windows when signal 0 isn’t supported, yet this code treats every OSError as “parent vanished” and forces shutdown. Any Windows run will therefore exit immediately. Please gate on the errno, disable probing for the unsupported cases, and keep exiting only when we actually get ProcessLookupError.

@@
-import signal
-import sys
-import threading
+import errno
+import signal
+import sys
+import threading
@@
-                except ValueError:
-                    # Signal 0 unsupported on this platform (e.g., Windows); disable parent probing
-                    parent_pid = None
-                except (ProcessLookupError, OSError):
+                except ValueError:
+                    parent_pid = None
+                except ProcessLookupError:
                     logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
                     break
+                except OSError as exc:
+                    if exc.errno in (errno.EPERM, errno.EACCES, errno.EINVAL, errno.ENOSYS):
+                        logger.debug("Parent probe unsupported; disabling parent monitoring", exc_info=True)
+                        parent_pid = None
+                        continue
+                    logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
+                    break
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if parent_pid is not None:
try:
os.kill(parent_pid, 0)
except ValueError:
# Signal 0 unsupported on this platform (e.g., Windows); disable parent probing
parent_pid = None
except (ProcessLookupError, OSError):
logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
break
try:
if sys.stdin.closed:
logger.info("stdin.closed is True; client disconnected")
break
fd = sys.stdin.fileno()
if fd < 0:
logger.info("stdin fd invalid; client disconnected")
break
except (ValueError, OSError, AttributeError):
# Closed pipe or unavailable stdin
if parent_pid is not None:
try:
os.kill(parent_pid, 0)
except ValueError:
parent_pid = None
except ProcessLookupError:
logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
break
except OSError as exc:
if exc.errno in (errno.EPERM, errno.EACCES, errno.EINVAL, errno.ENOSYS):
logger.debug("Parent probe unsupported; disabling parent monitoring", exc_info=True)
parent_pid = None
continue
logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
break
try:
if sys.stdin.closed:
logger.info("stdin.closed is True; client disconnected")
break
fd = sys.stdin.fileno()
if fd < 0:
logger.info("stdin fd invalid; client disconnected")
break
except (ValueError, OSError, AttributeError):
# Closed pipe or unavailable stdin
🤖 Prompt for AI Agents 
In MCPForUnity/UnityMcpServer~/src/server.py around lines 217-236, the
parent-probing code treats all OSErrors as "parent vanished" which causes
immediate exits on Windows; change the exception handling so PermissionError and
OSError with errno indicating signal-0 unsupported (e.g., errno.EINVAL or
errno.EPERM) do not trigger shutdown but instead disable parent probing by
setting parent_pid = None, while only ProcessLookupError should log "parent no
longer exists" and break; use the errno module to inspect e.errno and keep the
existing ValueError handling as-is.

break
except Exception:
# Ignore transient errors
logger.debug("Transient error checking stdin", exc_info=True)

if not _shutdown_flag.is_set():
logger.info("Client disconnected (stdin or parent), initiating shutdown...")
_shutdown_flag.set()
if not _exit_timer_scheduled.is_set():
_exit_timer_scheduled.set()
threading.Timer(0.5, _force_exit, args=(0,)).start()
except Exception:
# Never let monitor thread crash the process
logger.debug("Monitor thread error", exc_info=True)


def main():
"""Entry point for uvx and console scripts."""
mcp.run(transport='stdio')
try:
signal.signal(signal.SIGTERM, _signal_handler)
signal.signal(signal.SIGINT, _signal_handler)
if hasattr(signal, "SIGPIPE"):
signal.signal(signal.SIGPIPE, signal.SIG_IGN)
if hasattr(signal, "SIGBREAK"):
signal.signal(signal.SIGBREAK, _signal_handler)
except Exception:
# Signals can fail in some environments
pass

t = threading.Thread(target=_monitor_stdin, daemon=True)
t.start()

try:
mcp.run(transport='stdio')
logger.info("FastMCP run() returned (stdin EOF or disconnect)")
except (KeyboardInterrupt, SystemExit):
logger.info("Server interrupted; shutting down")
_shutdown_flag.set()
except BrokenPipeError:
logger.info("Broken pipe; shutting down")
_shutdown_flag.set()
except Exception as e:
logger.error(f"Server error: {e}", exc_info=True)
_shutdown_flag.set()
_force_exit(1)
finally:
_shutdown_flag.set()
logger.info("Server main loop exited")
if not _exit_timer_scheduled.is_set():
_exit_timer_scheduled.set()
threading.Timer(0.5, _force_exit, args=(0,)).start()


# Run the server
Expand Down
98 changes: 97 additions & 1 deletion Server/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
from logging.handlers import RotatingFileHandler
import os
from contextlib import asynccontextmanager
import sys
import signal
import threading
from typing import AsyncIterator, Dict, Any
from config import config
from tools import register_all_tools
Expand Down Expand Up @@ -64,6 +67,10 @@
# Global connection state
_unity_connection: UnityConnection = None

# Global shutdown coordination
_shutdown_flag = threading.Event()
_exit_timer_scheduled = threading.Event()


@asynccontextmanager
async def server_lifespan(server: FastMCP) -> AsyncIterator[Dict[str, Any]]:
Expand Down Expand Up @@ -186,9 +193,98 @@ def _emit_startup():
register_all_resources(mcp)


def _force_exit(code: int = 0):
"""Force process exit, bypassing any background threads that might linger."""
os._exit(code)


def _signal_handler(signum, frame):
logger.info(f"Received signal {signum}, initiating shutdown...")
_shutdown_flag.set()
if not _exit_timer_scheduled.is_set():
_exit_timer_scheduled.set()
threading.Timer(1.0, _force_exit, args=(0,)).start()


def _monitor_stdin():
"""Background thread to detect stdio detach (stdin EOF) or parent exit."""
try:
parent_pid = os.getppid() if hasattr(os, "getppid") else None
while not _shutdown_flag.is_set():
if _shutdown_flag.wait(0.5):
break

if parent_pid is not None:
try:
os.kill(parent_pid, 0)
except ValueError:
# Signal 0 unsupported on this platform (e.g., Windows); disable parent probing
parent_pid = None
except (ProcessLookupError, OSError):
logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
break

try:
if sys.stdin.closed:
logger.info("stdin.closed is True; client disconnected")
break
fd = sys.stdin.fileno()
if fd < 0:
logger.info("stdin fd invalid; client disconnected")
break
except (ValueError, OSError, AttributeError):
# Closed pipe or unavailable stdin
Comment on lines +217 to +236
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 1, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard against unsupported signal 0 on Windows.

Here too, os.kill(parent_pid, 0) will raise PermissionError/EINVAL on Windows whenever signal 0 isn’t supported. Treating all OSError values like ProcessLookupError means the server kills itself immediately even though the parent is fine. Please special‑case those errnos, disable parent probing, and only shut down when we genuinely see ProcessLookupError.

@@
-import signal
-import sys
-import threading
+import errno
+import signal
+import sys
+import threading
@@
-                except ValueError:
-                    # Signal 0 unsupported on this platform (e.g., Windows); disable parent probing
-                    parent_pid = None
-                except (ProcessLookupError, OSError):
+                except ValueError:
+                    parent_pid = None
+                except ProcessLookupError:
                     logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
                     break
+                except OSError as exc:
+                    if exc.errno in (errno.EPERM, errno.EACCES, errno.EINVAL, errno.ENOSYS):
+                        logger.debug("Parent probe unsupported; disabling parent monitoring", exc_info=True)
+                        parent_pid = None
+                        continue
+                    logger.info(f"Parent process {parent_pid} no longer exists; shutting down")
+                    break

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In Server/server.py around lines 217-236, the except handling for
os.kill(parent_pid, 0) treats all OSError the same and can cause premature
shutdown on Windows; change it to special-case signal-not-supported errors by
importing errno and checking the exception errno: if the caught exception is
PermissionError or has errno in (errno.EINVAL, errno.EPERM) then set parent_pid
= None (disable parent probing) and continue, but only log and break for
ProcessLookupError or errors with errno == errno.ESRCH that indicate the parent
truly doesn't exist; re-raise or handle other unexpected errors appropriately.

break
except Exception:
# Ignore transient errors
logger.debug("Transient error checking stdin", exc_info=True)

if not _shutdown_flag.is_set():
logger.info("Client disconnected (stdin or parent), initiating shutdown...")
_shutdown_flag.set()
if not _exit_timer_scheduled.is_set():
_exit_timer_scheduled.set()
threading.Timer(0.5, _force_exit, args=(0,)).start()
except Exception:
# Never let monitor thread crash the process
logger.debug("Monitor thread error", exc_info=True)


def main():
"""Entry point for uvx and console scripts."""
mcp.run(transport='stdio')
try:
signal.signal(signal.SIGTERM, _signal_handler)
signal.signal(signal.SIGINT, _signal_handler)
if hasattr(signal, "SIGPIPE"):
signal.signal(signal.SIGPIPE, signal.SIG_IGN)
if hasattr(signal, "SIGBREAK"):
signal.signal(signal.SIGBREAK, _signal_handler)
except Exception:
# Signals can fail in some environments
pass

t = threading.Thread(target=_monitor_stdin, daemon=True)
t.start()

try:
mcp.run(transport='stdio')
logger.info("FastMCP run() returned (stdin EOF or disconnect)")
except (KeyboardInterrupt, SystemExit):
logger.info("Server interrupted; shutting down")
_shutdown_flag.set()
except BrokenPipeError:
logger.info("Broken pipe; shutting down")
_shutdown_flag.set()
except Exception as e:
logger.error(f"Server error: {e}", exc_info=True)
_shutdown_flag.set()
_force_exit(1)
finally:
_shutdown_flag.set()
logger.info("Server main loop exited")
if not _exit_timer_scheduled.is_set():
_exit_timer_scheduled.set()
threading.Timer(0.5, _force_exit, args=(0,)).start()


# Run the server
Expand Down
Loading