Skip to content

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

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

Open
webtecnica wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eac6c3c
fix(media): serve and render .md files with inline Markdown preview
webtecnica Jul 18, 2026
b4d0c47
fix: add SIGCHLD reaper to prevent zombie defunct processes (#6145)
webtecnica Jul 19, 2026
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
3 changes: 3 additions & 0 deletions api/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,9 @@ def verify_hermes_imports() -> tuple:
".json": "application/json",
".html": "text/html",
".htm": "text/html",
".md": "text/markdown",
".mkd": "text/markdown",
".mkdn": "text/markdown",
".xls": "application/vnd.ms-excel",
".xlsx": "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
".doc": "application/msword",
Expand Down
2 changes: 1 addition & 1 deletion api/routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18767,7 +18767,7 @@ def _handle_media(handler, parsed):
"video/mp4", "video/quicktime", "video/webm", "video/ogg",
"application/pdf",
}
_SESSION_MEDIA_TOKEN_TYPES = _INLINE_IMAGE_TYPES | _AUDIO_VIDEO_PDF_TYPES | {"text/html"}
_SESSION_MEDIA_TOKEN_TYPES = _INLINE_IMAGE_TYPES | _AUDIO_VIDEO_PDF_TYPES | {"text/html", "text/markdown"}
session_media_allowed = _session_media_token_allows_path(
qs.get("session_id", [""])[0],
target,
Expand Down
24 changes: 24 additions & 0 deletions server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@
if _SIGPIPE is not None:
signal.signal(_SIGPIPE, signal.SIG_IGN)


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
Comment on lines +32 to +38

@greptile-apps greptile-apps Bot Jul 19, 2026

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.



signal.signal(signal.SIGCHLD, _reap_children)

@greptile-apps greptile-apps Bot Jul 19, 2026

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 on lines +20 to +41

@greptile-apps greptile-apps Bot Jul 19, 2026

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!


# Test-mode network isolation keeps subprocess-backed tests hermetic.
if os.environ.get("HERMES_WEBUI_TEST_NETWORK_BLOCK", "").strip() in ("1", "true", "yes"):
_REAL_CREATE_CONN = socket.create_connection
Expand Down
11 changes: 11 additions & 0 deletions static/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6582,6 +6582,17 @@ main.main > .main-view:not([id="mainChat"]):not([id="mainSettings"]) .main-view-
.html-preview-fallback{padding:8px;font-size:13px;}
.html-preview-spinner{animation:pulse 1.5s ease-in-out infinite;}

/* ── Markdown inline preview ──────────────────────────────────────────────── */
.md-inline-load{color:var(--muted);font-size:13px;padding:8px 12px;border:1px dashed var(--border);border-radius:8px;margin:6px 0;}
.md-preview-spinner{animation:pulse 1.5s ease-in-out infinite;}
.md-inline-wrap{border:1px solid var(--border);border-radius:6px;overflow:hidden;margin:4px 0;background:var(--surface);}
.md-inline-header{display:flex;justify-content:space-between;align-items:center;padding:6px 10px;background:var(--surface-2);font-size:12px;font-weight:600;border-bottom:1px solid var(--border);}
.md-preview-title{color:var(--text);}
.md-inline-fallback{padding:8px;font-size:13px;}
.md-inline-content{padding:12px 16px;overflow-x:auto;}
.md-inline-content > :first-child{margin-top:0;}
.md-inline-content > :last-child{margin-bottom:0;}

/* ── Insights panel (#464) ────────────────────────────────────────────────── */
main.main.showing-insights > #mainInsights{display:flex;overflow-y:auto;}
.insights-grid{display:grid;grid-template-columns:repeat(auto-fit,minmax(140px,1fr));gap:12px;margin-bottom:16px;}
Expand Down
33 changes: 33 additions & 0 deletions static/ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2499,6 +2499,7 @@ const _SVG_EXTS=/\.svg$/i;
const _AUDIO_EXTS=/\.(mp3|ogg|wav|m4a|aac|flac|wma|opus|webm|oga)$/i;
const _VIDEO_EXTS=/\.(mp4|webm|mkv|mov|avi|ogv|m4v)$/i;
const _CSV_EXTS=/\.csv$/i;
const _MD_EXTS=/\.(md|mkd|mkdn)$/i;
const _EXCALIDRAW_EXTS=/\.excalidraw$/i;
// ── Media playback speed controls ─────────────────────────────────────────
const MEDIA_PLAYBACK_RATES=[0.5,0.75,1,1.25,1.5,2];
Expand Down Expand Up @@ -2661,6 +2662,9 @@ function _inlineMediaHtmlForRef(ref, sessionId, altText){
if(_HTML_EXTS.test(ref)){
return `<div class="html-preview-load" data-path="${esc(ref)}"><span class="html-preview-spinner">⏳</span> ${esc(typeof t==='function'?t('html_loading'):'Loading')}...</div>`;
}
if(_MD_EXTS.test(ref)){
return `<div class="md-inline-load" data-path="${esc(ref)}"><span class="md-preview-spinner">⏳</span> ${esc(typeof t==='function'?t('md_loading'):'Loading')}...</div>`;
}
const fname=esc(ref.split('/').pop()||ref);
if(/\.(patch|diff)$/i.test(ref)) return `<div class="diff-inline-load" data-path="${esc(ref)}">${esc(typeof t==='function'?t('diff_loading'):'Loading diff')} ${fname}...</div>`;
if(_CSV_EXTS.test(ref)) return `<div class="csv-inline-load" data-path="${esc(ref)}">${esc(typeof t==='function'?t('csv_loading'):'Loading')} ${fname}...</div>`;
Expand Down Expand Up @@ -18087,6 +18091,7 @@ function postProcessRenderedMessages(container) {
loadExcalidrawInline(container);
loadPdfInline(container);
loadHtmlInline(container);
loadMarkdownInline(container);
renderMermaidBlocks(container);
renderKatexBlocks(container);
initTreeViews(container);
Expand Down Expand Up @@ -18654,6 +18659,34 @@ function loadHtmlInline(container){
});
}

const MD_MAX_SIZE = 256 * 1024;

function loadMarkdownInline(container){
const root = container || document;
root.querySelectorAll('.md-inline-load:not([data-loaded])').forEach(el => {
el.setAttribute('data-loaded', '1');
const path = el.dataset.path;
const fname = path.split('/').pop() || path;
const mediaSessionId = (typeof S !== 'undefined' && S && S.session && S.session.session_id) ? String(S.session.session_id) : '';
const publicMediaUrl = 'api/media?path=' + encodeURIComponent(path);
const mediaUrl = publicMediaUrl + (mediaSessionId ? '&session_id=' + encodeURIComponent(mediaSessionId) : '');
const downloadUrl = mediaUrl + '&download=1';
fetch(mediaUrl)
.then(r => { if (!r.ok) throw new Error(r.status); return r.text(); })
.then(text => {
if (text.length > MD_MAX_SIZE) {
el.outerHTML = `<div class="md-inline-fallback"><a class="msg-media-link" href="${esc(downloadUrl)}" download="${esc(fname)}">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${esc(typeof t === 'function' ? t('md_too_large') : 'File too large for preview')}</span></div>`;
return;
}
const rendered = renderMd(text);
el.outerHTML = `<div class="md-inline-wrap"><div class="md-inline-header"><span class="md-preview-title">${esc(fname)}</span><a class="msg-media-link" href="${esc(downloadUrl)}" download="${esc(fname)}">📎 ${esc(fname)}</a></div><div class="md-inline-content">${rendered}</div></div>`;
})
.catch(() => {
el.outerHTML = `<div class="md-inline-fallback"><a class="msg-media-link" href="${esc(downloadUrl)}" download="${esc(fname)}">📎 ${esc(fname)}</a><br><span style="color:var(--muted);font-size:12px">${esc(typeof t === 'function' ? t('md_error') : 'Error loading markdown')}</span></div>`;
});
});
}

function renderMermaidBlocks(container){
const root=container||document;
const blocks=root.querySelectorAll('.mermaid-block:not([data-rendered])');
Expand Down
Loading