Skip to content

fix(desktop): keep terminal-host daemon alive across app quit #3574

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
Haknt wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(desktop): keep terminal-host daemon alive across app quit #3574

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
8b593d2
fix(desktop): keep terminal-host daemon alive across app quit
Haknt Apr 19, 2026
920432c
fix(daemon): harden SIGHUP handler and clean up early guard
Haknt Apr 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
13 changes: 13 additions & 0 deletions apps/desktop/src/main/terminal-host/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,11 +822,24 @@ function setupSignalHandlers() {
// =============================================================================

async function main() {
// Belt-and-suspenders SIGHUP guard, installed before anything else (log
// calls included). Without a registered listener, Node's default action
// for SIGHUP is to terminate the process — so if Electron tears down the
// macOS login session during our startup logging, the daemon dies before
// setupSignalHandlers() can install the definitive listener.
//
// We remove this temporary guard after setupSignalHandlers() to keep the
// listener set clean — `process.on` adds rather than replaces, so leaving
// it in place would leave two SIGHUP listeners for the daemon's lifetime.
const earlyHupGuard = (): void => {};
process.on("SIGHUP", earlyHupGuard);

log("info", "Terminal Host Daemon starting...");
log("info", `Environment: ${process.env.NODE_ENV || "production"}`);
log("info", `Home directory: ${SUPERSET_HOME_DIR}`);

setupSignalHandlers();
process.removeListener("SIGHUP", earlyHupGuard);

try {
await startServer();
Expand Down
29 changes: 23 additions & 6 deletions apps/desktop/src/main/terminal-host/signal-handlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,30 @@ export function setupTerminalHostSignalHandlers({
timeoutMessage: "Forced exit after SIGTERM shutdown timeout",
});
});
// SIGHUP: intentionally ignored (nohup semantics).
//
// The daemon is spawned with `detached: true` + `child.unref()` so it can
// outlive Electron's exit — see marketing blog terminal-daemon-deep-dive
// "Terminal survives app restart". On macOS, `setsid()` isolates the Unix
// SID but the daemon still shares the Mach bootstrap / login Security
// Session with its parent. When Electron calls `app.exit(0)` and tears
// down that login session, SIGHUP propagates to the daemon and kills it
// unless explicitly ignored. Registering a no-op listener prevents the
// default terminate action without triggering our shutdownOnce path.
process.on("SIGHUP", () => {
shutdownOnce({
exitCode: 0,
message: "Received SIGHUP, shutting down...",
stopServerErrorMessage: "Error during stopServer in SIGHUP shutdown",
timeoutMessage: "Forced exit after SIGHUP shutdown timeout",
});
// Protect the nohup semantics even if logging fails. If the daemon's
// stdout/stderr pipe closes (e.g. Electron exits first and we were
// spawned with stdio:"pipe"/"inherit"), writing to it throws EPIPE
// — an uncaught exception would flow into shutdownOnce and defeat
// the entire purpose of this handler.
try {
log(
"info",
"Received SIGHUP; ignoring (nohup semantics for daemon survival)",
);
} catch {
// Preserve ignore semantics unconditionally.
}
});
Comment on lines 140 to 154
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

P1 Unprotected log() call inside signal handler

If the daemon's stdout/stderr is still connected to Electron's pipe (i.e., spawned with stdio: 'pipe' or stdio: 'inherit' rather than stdio: 'ignore'), and Electron exits first, writing to that closed pipe will throw EPIPE. Because the log() call here is not wrapped in a try/catch, an EPIPE will propagate as an uncaught exception into the uncaughtException handler. EPIPE is not in TRANSIENT_ERROR_CODES, so shutdownOnce() would be called with exitCode: 1 — defeating the entire purpose of this no-op handler.

If stdio is already redirected to a file or /dev/null at spawn time, this is a non-issue. But it is worth adding a guard to make the handler unconditionally safe:

process.on("SIGHUP", () => {
    try {
        log("info", "Received SIGHUP; ignoring (nohup semantics for daemon survival)");
    } catch {
        // Suppress logging errors (e.g. EPIPE after parent exit) —
        // the whole point of this handler is to survive without side-effects.
    }
});
Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/main/terminal-host/signal-handlers.ts
Line: 140-142

Comment:
**Unprotected `log()` call inside signal handler**

If the daemon's stdout/stderr is still connected to Electron's pipe (i.e., spawned with `stdio: 'pipe'` or `stdio: 'inherit'` rather than `stdio: 'ignore'`), and Electron exits first, writing to that closed pipe will throw `EPIPE`. Because the `log()` call here is not wrapped in a `try/catch`, an EPIPE will propagate as an uncaught exception into the `uncaughtException` handler. `EPIPE` is not in `TRANSIENT_ERROR_CODES`, so `shutdownOnce()` would be called with `exitCode: 1` — defeating the entire purpose of this no-op handler.

If stdio is already redirected to a file or `/dev/null` at spawn time, this is a non-issue. But it is worth adding a guard to make the handler unconditionally safe:

```ts
process.on("SIGHUP", () => {
    try {
        log("info", "Received SIGHUP; ignoring (nohup semantics for daemon survival)");
    } catch {
        // Suppress logging errors (e.g. EPIPE after parent exit) —
        // the whole point of this handler is to survive without side-effects.
    }
});
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Author

@Haknt Haknt Apr 19, 2026

Choose a reason for hiding this comment

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

Addressed in 903c94b93. Wrapped the log() call in try/catch. If the pipe closes first (e.g. Electron exits and our stdio was inherited), EPIPE from the write no longer propagates to uncaughtException → shutdownOnce, preserving the nohup semantics unconditionally.

Comment thread
coderabbitai[bot] marked this conversation as resolved.

process.on("uncaughtException", (error) => {
Expand Down