From 4c81fbed2b5d5262d86cfb0df55cd7b32d438101 Mon Sep 17 00:00:00 2001 From: songlei Date: Mon, 27 Apr 2026 17:47:30 +0800 Subject: [PATCH] fix(daemon/windows): break out of parent shell Job Object so daemon survives Approved and merged via Multica after CI passed. --- server/cmd/multica/cmd_daemon.go | 46 ++++++++++++++++---- server/cmd/multica/cmd_daemon_unix.go | 13 +++++- server/cmd/multica/cmd_daemon_windows.go | 53 +++++++++++++++++++----- 3 files changed, 94 insertions(+), 18 deletions(-) diff --git a/server/cmd/multica/cmd_daemon.go b/server/cmd/multica/cmd_daemon.go index c8833bf0b..fc5fbd82a 100644 --- a/server/cmd/multica/cmd_daemon.go +++ b/server/cmd/multica/cmd_daemon.go @@ -174,11 +174,29 @@ func runDaemonBackground(cmd *cobra.Command) error { child := exec.Command(exePath, args...) child.Stdout = logFile child.Stderr = logFile - child.SysProcAttr = daemonSysProcAttr() + // On Windows we want to break the child out of the parent shell's Job + // Object so the daemon survives parent-shell exit. If the parent's Job + // has not granted BREAKAWAY_OK, CreateProcess returns + // ERROR_ACCESS_DENIED — fall back to spawning without breakaway, which + // matches the pre-fix behaviour. On Unix the bool is a no-op. + child.SysProcAttr = daemonSysProcAttr(true) if err := child.Start(); err != nil { - logFile.Close() - return fmt.Errorf("start daemon: %w", err) + if isAccessDeniedSpawnErr(err) { + // Retry without breakaway. Reset the cmd state — exec.Cmd is + // not safe to Start() twice, so build a fresh one. + child = exec.Command(exePath, args...) + child.Stdout = logFile + child.Stderr = logFile + child.SysProcAttr = daemonSysProcAttr(false) + if err := child.Start(); err != nil { + logFile.Close() + return fmt.Errorf("start daemon (no breakaway): %w", err) + } + } else { + logFile.Close() + return fmt.Errorf("start daemon: %w", err) + } } logFile.Close() pid := child.Process.Pid @@ -327,12 +345,26 @@ func runDaemonForeground(cmd *cobra.Command) error { } child.Stdout = logFile child.Stderr = logFile - child.SysProcAttr = daemonSysProcAttr() + // Break out of the parent's Job Object on Windows; see the + // runDaemonBackground call site for rationale. + child.SysProcAttr = daemonSysProcAttr(true) if err := child.Start(); err != nil { - logFile.Close() - logger.Error("failed to start new daemon", "error", err) - return nil + if isAccessDeniedSpawnErr(err) { + child = exec.Command(restartBin, args...) + child.Stdout = logFile + child.Stderr = logFile + child.SysProcAttr = daemonSysProcAttr(false) + if err := child.Start(); err != nil { + logFile.Close() + logger.Error("failed to start new daemon (no breakaway)", "error", err) + return nil + } + } else { + logFile.Close() + logger.Error("failed to start new daemon", "error", err) + return nil + } } logFile.Close() child.Process.Release() diff --git a/server/cmd/multica/cmd_daemon_unix.go b/server/cmd/multica/cmd_daemon_unix.go index 9a84648b7..5c2a99c23 100644 --- a/server/cmd/multica/cmd_daemon_unix.go +++ b/server/cmd/multica/cmd_daemon_unix.go @@ -11,10 +11,21 @@ import ( "syscall" ) -func daemonSysProcAttr() *syscall.SysProcAttr { +// daemonSysProcAttr returns the attributes used when spawning the background +// daemon. The withBreakaway argument exists only to share a signature with +// the Windows version (where it controls CREATE_BREAKAWAY_FROM_JOB); on +// Unix Setsid alone is sufficient to detach the child from its parent's +// session and process group. +func daemonSysProcAttr(_ bool) *syscall.SysProcAttr { return &syscall.SysProcAttr{Setsid: true} } +// isAccessDeniedSpawnErr is always false on Unix. The Windows version +// looks for ERROR_ACCESS_DENIED to detect "parent Job Object disallowed +// breakaway" and trigger the breakaway-disabled retry; that retry is a +// no-op on Unix. +func isAccessDeniedSpawnErr(_ error) bool { return false } + func notifyShutdownContext(parent context.Context) (context.Context, context.CancelFunc) { return signal.NotifyContext(parent, syscall.SIGINT, syscall.SIGTERM) } diff --git a/server/cmd/multica/cmd_daemon_windows.go b/server/cmd/multica/cmd_daemon_windows.go index 6836bfb6e..c90bbc865 100644 --- a/server/cmd/multica/cmd_daemon_windows.go +++ b/server/cmd/multica/cmd_daemon_windows.go @@ -4,6 +4,7 @@ package main import ( "context" + "errors" "io" "os" "os/signal" @@ -12,25 +13,57 @@ import ( ) const ( + // detachedProcess severs the inherited console so closing the parent + // cmd/PowerShell window no longer propagates CTRL_CLOSE_EVENT to the daemon. detachedProcess = 0x00000008 - sigBreak = syscall.Signal(0x15) + // createBreakawayFromJob lets the daemon escape its parent shell's Job + // Object. Modern Windows Terminal / cmd.exe / PowerShell host the + // processes they spawn inside a Job Object that has KILL_ON_JOB_CLOSE + // set, so when the parent shell exits the kernel kills every process + // inside that job — including a child we tried to "detach" with + // detachedProcess alone. detachedProcess only severs the console, not + // the Job Object inheritance. Adding createBreakawayFromJob makes + // CreateProcess place the new process outside the parent's Job, so + // the daemon survives parent-shell exit. + // + // If the parent's Job has not granted BREAKAWAY_OK, CreateProcess + // returns ERROR_ACCESS_DENIED. In that case the caller falls back to + // detachedProcess alone — the daemon is then at the mercy of the + // parent's Job lifecycle, which is the pre-fix behaviour. + createBreakawayFromJob = 0x01000000 + sigBreak = syscall.Signal(0x15) ) // daemonSysProcAttr returns the attributes used when spawning the background -// daemon. DETACHED_PROCESS severs the inherited console so closing the parent -// cmd/PowerShell window no longer propagates CTRL_CLOSE_EVENT to the daemon. -// Because the detached daemon shares no console with the stop caller, -// `daemon stop` talks to it via the HTTP /shutdown endpoint rather than -// GenerateConsoleCtrlEvent. The daemon's stdout/stderr are already -// redirected to the log file before Start() is called, so losing the -// console is safe. -func daemonSysProcAttr() *syscall.SysProcAttr { +// daemon. The default is detachedProcess + createBreakawayFromJob so the +// daemon survives both the parent's console close and the parent's Job +// Object close. The daemon's stdout/stderr are already redirected to the +// log file before Start() is called, so losing the console is safe; and +// `daemon stop` talks to it via HTTP /shutdown rather than +// GenerateConsoleCtrlEvent, so losing the process group is also safe. +// +// The withBreakaway argument exists so the caller can retry with +// withBreakaway=false when CreateProcess fails with ERROR_ACCESS_DENIED +// (the parent Job does not allow breakaway). +func daemonSysProcAttr(withBreakaway bool) *syscall.SysProcAttr { + flags := uint32(detachedProcess) + if withBreakaway { + flags |= createBreakawayFromJob + } return &syscall.SysProcAttr{ HideWindow: true, - CreationFlags: detachedProcess, + CreationFlags: flags, } } +// isAccessDeniedSpawnErr reports whether the error returned from +// (*exec.Cmd).Start() is the Windows ERROR_ACCESS_DENIED, which is what +// CreateProcess returns when CREATE_BREAKAWAY_FROM_JOB is requested but +// the parent's Job Object has not set JOB_OBJECT_LIMIT_BREAKAWAY_OK. +func isAccessDeniedSpawnErr(err error) bool { + return errors.Is(err, syscall.ERROR_ACCESS_DENIED) +} + func notifyShutdownContext(parent context.Context) (context.Context, context.CancelFunc) { return signal.NotifyContext(parent, os.Interrupt, sigBreak) }