Skip to content

fix: add SIGCHLD reaper to prevent zombie defunct processes (#6145)#6284

Open
webtecnica wants to merge 2 commits into
nesquena:masterfrom
webtecnica:fix/6145-zombie-processes
Open

fix: add SIGCHLD reaper to prevent zombie defunct processes (#6145)#6284
webtecnica wants to merge 2 commits into
nesquena:masterfrom
webtecnica:fix/6145-zombie-processes

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

Summary

Adds a global SIGCHLD signal handler in server.py that reaps any exited child processes, preventing [hermes] <defunct> zombie accumulation under hermes-webui.service.

Problem

When hermes gateway restart is triggered from the WebUI and takes >2s, gateway_restart.py delegates reaping to a daemon thread. If that thread is lost (server reload, exception, etc.), the child is never reaped — becoming a zombie with PPID = server.py. The only way to clear it is restarting the service.

The root cause is that server.py had no SIGCHLD handler — only SIGPIPE and SIGTERM were handled.

Fix

Adds _reap_children() in server.py (next to the existing SIGPIPE handler):

def _reap_children(_signo, _frame):
    try:
        while True:
            pid, _status = os.waitpid(-1, os.WNOHANG)
            if pid == 0:
                break
    except ChildProcessError:
        pass

signal.signal(signal.SIGCHLD, _reap_children)
  • Uses waitpid(-1, WNOHANG)non-blocking, only collects already-exited children
  • Never disturbs running subprocesses
  • On modern CPython, subprocess.wait() tolerates an already-reaped child via ChildProcessError
  • Global safety net for all async spawns: gateway restarts, terminal runners, plugin workers

Closes #6145.

Closes common issue where clicking MEDIA: links for .md files could
download a JSON auth error on desktop. Three-layer fix:

1. MIME mapping (api/config.py): add .md/.mkd/.mkdn → text/markdown
   so the server returns the correct Content-Type instead of
   application/octet-stream

2. Session allow-list (api/routes.py): add text/markdown to
   _SESSION_MEDIA_TOKEN_TYPES so .md files are accepted by the
   session-token path check

3. Frontend preview (static/ui.js + static/style.css): detect .md
   files in _inlineMediaHtmlForRef, fetch content via api/media,
   render through the existing renderMd() pipeline, and display
   inline in a styled container — same pattern as HTML/CSV previews
…#6145)

Add a global SIGCHLD signal handler in server.py that reaps any exited
child processes using os.waitpid(-1, os.WNOHANG). This prevents zombie
[hermes] <defunct> processes from accumulating under hermes-webui.service
when asynchronous subprocess spawners (like gateway restart) lose their
daemon-thread waiters.

The handler is non-blocking and only collects already-exited children,
so it never disturbs running subprocesses. On modern CPython,
subprocess.wait() tolerates an already-reaped child via ChildProcessError
and stays correct.

Closes nesquena#6145
@greptile-apps

greptile-apps Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR primarily adds a global SIGCHLD handler in server.py to reap zombie child processes left behind when the gateway-restart daemon thread is lost. It also bundles an unrelated markdown inline preview feature across api/config.py, api/routes.py, static/style.css, and static/ui.js.

  • The _reap_children handler uses os.waitpid(-1, os.WNOHANG) in a loop, which is the standard zombie-reaping pattern, but it races with subprocess.Popen.wait() calls in gateway_restart.py; CPython resolves the resulting ChildProcessError (ECHILD) by defaulting returncode to 0, so a failed gateway restart can be silently reported as success.
  • The markdown preview (loadMarkdownInline) fetches file content and passes it through the existing renderMd() sanitizer, which correctly strips unsafe tags via _tag() — no XSS exposure was found.
  • signal.SIGCHLD is registered unconditionally, unlike the SIGPIPE handler above it which is safely guarded with getattr; this would raise AttributeError on Windows.

Confidence Score: 3/5

Not safe to merge as-is — the SIGCHLD handler can cause failed gateway restarts to be silently reported as successful.

The _reap_children handler discards the child exit status and races with subprocess.Popen.wait() in gateway_restart.py. When the handler wins, CPython defaults proc.returncode to 0, so a non-zero exit from hermes gateway restart would pass the proc.returncode == 0 check and return "status": "completed" to the caller — masking the actual failure. The markdown preview additions are clean. The unguarded signal.SIGCHLD registration is a portability gap consistent with the defensive style used for SIGPIPE.

server.py (SIGCHLD handler implementation) and api/gateway_restart.py (return-code logic at line 133 is the affected consumer)

Important Files Changed

Filename Overview
server.py Adds a global SIGCHLD handler to reap zombie children; the waitpid(-1, WNOHANG) pattern races with subprocess.Popen.wait() and can lose exit-code information
api/gateway_restart.py No changes in this PR, but it is the primary caller affected by the new SIGCHLD handler — returncode-based status detection can be broken by the race
api/config.py Adds .md / .mkd / .mkdn MIME type entries; straightforward and correct
api/routes.py Adds text/markdown to the session media token allowlist so markdown files can be served through the media endpoint
static/ui.js Adds loadMarkdownInline() which fetches and renders markdown files through the existing renderMd() sanitizer; the existing _tag() sanitizer mitigates XSS from malicious file content
static/style.css Adds CSS classes for markdown inline preview; clean and self-contained

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant MainThread as Main Thread (server.py)
    participant WorkerThread as Worker Thread (gateway_restart.py)
    participant OS as OS / Kernel

    WorkerThread->>OS: subprocess.Popen(hermes gateway restart)
    OS-->>WorkerThread: "proc (pid=X)"

    Note over WorkerThread: proc.communicate(timeout=2s) or proc.wait(timeout=240s)

    WorkerThread->>OS: os.waitpid(X, 0) [blocking]

    OS-->>OS: "Process X exits (e.g. rc=1)"
    OS->>MainThread: SIGCHLD delivered

    MainThread->>OS: os.waitpid(-1, WNOHANG) [_reap_children]
    OS-->>MainThread: "(X, status=1) — status discarded as _status"

    Note over OS: Zombie for X is now gone

    OS-->>WorkerThread: ECHILD (no such child)
    WorkerThread->>WorkerThread: "except ChildProcessError: sts=0"
    WorkerThread->>WorkerThread: "proc.returncode = 0  WRONG"

    WorkerThread-->>WorkerThread: status: completed reported (should be: failed)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant MainThread as Main Thread (server.py)
    participant WorkerThread as Worker Thread (gateway_restart.py)
    participant OS as OS / Kernel

    WorkerThread->>OS: subprocess.Popen(hermes gateway restart)
    OS-->>WorkerThread: "proc (pid=X)"

    Note over WorkerThread: proc.communicate(timeout=2s) or proc.wait(timeout=240s)

    WorkerThread->>OS: os.waitpid(X, 0) [blocking]

    OS-->>OS: "Process X exits (e.g. rc=1)"
    OS->>MainThread: SIGCHLD delivered

    MainThread->>OS: os.waitpid(-1, WNOHANG) [_reap_children]
    OS-->>MainThread: "(X, status=1) — status discarded as _status"

    Note over OS: Zombie for X is now gone

    OS-->>WorkerThread: ECHILD (no such child)
    WorkerThread->>WorkerThread: "except ChildProcessError: sts=0"
    WorkerThread->>WorkerThread: "proc.returncode = 0  WRONG"

    WorkerThread-->>WorkerThread: status: completed reported (should be: failed)
Loading

Reviews (1): Last reviewed commit: "fix: add SIGCHLD reaper to prevent zombi..." | Re-trigger Greptile

Comment thread server.py
Comment on lines +32 to +38
try:
while True:
pid, _status = os.waitpid(-1, os.WNOHANG)
if pid == 0:
break
except ChildProcessError:
pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 SIGCHLD reaper races with subprocess.Popen.wait() and silently zeros returncode

When _reap_children calls os.waitpid(-1, os.WNOHANG) and wins the race against an in-flight proc.wait() (e.g. the daemon thread in gateway_restart.py), CPython's subprocess._try_wait catches ChildProcessError (ECHILD) and returns sts = 0, causing proc.returncode to be set to 0 regardless of the actual exit code. The restart_active_profile_gateway caller then evaluates proc.returncode == 0 at line 133 and returns "status": "completed" — silently converting a failed restart into a reported success. The discarded _status local in this handler is the lost signal.

Comment thread server.py
pass


signal.signal(signal.SIGCHLD, _reap_children)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 signal.SIGCHLD is not guarded for non-POSIX platforms, unlike the SIGPIPE handler directly above it. On Windows, signal.SIGCHLD does not exist and this line raises AttributeError at import time. The existing SIGPIPE pattern (using getattr) is the right defensive idiom to follow here.

Suggested change
signal.signal(signal.SIGCHLD, _reap_children)
_SIGCHLD = getattr(signal, "SIGCHLD", None)
if _SIGCHLD is not None:
signal.signal(_SIGCHLD, _reap_children)

Comment thread server.py
Comment on lines +20 to +41
def _reap_children(_signo, _frame):
"""Reap any exited child processes so they never linger as zombies.

Uses ``os.waitpid(-1, os.WNOHANG)`` in a loop to collect every already-exited
child in a single signal delivery. The handler is non-blocking and never
disturbs running subprocesses. On modern CPython ``subprocess.wait()``
tolerates an already-reaped child via ``ChildProcessError`` and stays correct.

This is a global safety net for every asynchronous spawn in the WebUI process
— including gateway restarts, terminal runners, and plugin workers — so that
a lost daemon-thread waiter can never leak a zombie.
"""
try:
while True:
pid, _status = os.waitpid(-1, os.WNOHANG)
if pid == 0:
break
except ChildProcessError:
pass


signal.signal(signal.SIGCHLD, _reap_children)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Two unrelated features bundled in one PR

server.py adds the SIGCHLD zombie reaper, but api/config.py, api/routes.py, static/style.css, and static/ui.js add a full markdown inline preview feature that has no relationship to zombie process management. AGENTS.md explicitly requires "Keep one logical change per PR; split unrelated refactors or cleanup." The markdown preview work should be split into its own PR.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@cutter-sh

cutter-sh Bot commented Jul 19, 2026

Copy link
Copy Markdown

🎬 Cutter preview — PR #6284

/chat
/chat — Markdown files now render inline in chat with a titled preview card and download link.

@nesquena-hermes nesquena-hermes added the size:M Medium PR (≤10 files, ≤250 LOC) label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M Medium PR (≤10 files, ≤250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zombie [hermes] <defunct> processes accumulate under hermes-webui.service (no SIGCHLD reaper)

2 participants