diff --git a/main.go b/main.go index 50114a9..c52d1ca 100644 --- a/main.go +++ b/main.go @@ -79,6 +79,10 @@ func acquireInstance(wait bool) bool { return false } +// postExitSettle is the pause between the previous instance ending and this +// one claiming its place. 400 ms, the figure the PowerShell helper used. +const postExitSettle = 400 * time.Millisecond + // waitPidArg reads "--wait-pid N": the process this one must outlive. func waitPidArg(args []string) int { for i, a := range args { @@ -131,6 +135,17 @@ func main() { } else { bootLog("the previous instance (pid %d) is STILL running after 60s — trying anyway", pid) } + // A breath after it is gone, which the PowerShell helper took as + // "Start-Sleep -Milliseconds 400" and the direct launch dropped. + // + // A process's handles are released by the kernel as it dies, so the + // mutex is free the instant the wait returns — but the things AROUND + // it are not on that clock: the WebView2 user-data lock, the log file, + // an antivirus that woke up when the exe was replaced. The old code + // waited here and worked; this is not a theory about which of those it + // was, it is the pause being put back. + time.Sleep(postExitSettle) + bootLog("settled %v after the previous instance — taking the lock", postExitSettle) } // A self-relaunch (database switch) races its own parent: the new process // regularly wins the start against the old one's teardown, and the operator diff --git a/proc_detach_other.go b/proc_detach_other.go new file mode 100644 index 0000000..f8f668e --- /dev/null +++ b/proc_detach_other.go @@ -0,0 +1,9 @@ +//go:build !windows + +package main + +import "os/exec" + +// detachProcess is a no-op off Windows: the relaunch there is an ordinary fork +// and nothing is inherited that needs breaking. +func detachProcess(cmd *exec.Cmd) {} diff --git a/proc_detach_windows.go b/proc_detach_windows.go new file mode 100644 index 0000000..df3bf0a --- /dev/null +++ b/proc_detach_windows.go @@ -0,0 +1,27 @@ +//go:build windows + +package main + +import ( + "os/exec" + "syscall" +) + +// detachProcess starts the child on its own, the way PowerShell's Start-Process +// left the relaunched OpsLog. +// +// DETACHED_PROCESS gives it no console of ours, CREATE_NEW_PROCESS_GROUP takes +// it out of our group so a Ctrl-Break or a job-object cleanup aimed at the old +// instance cannot reach the new one. Deliberately NOT CREATE_NO_WINDOW and NOT +// HideWindow: SW_HIDE in the STARTUPINFO is what made the updated OpsLog start +// invisibly, and there is no console to suppress — OpsLog is linked for the +// Windows GUI subsystem. +func detachProcess(cmd *exec.Cmd) { + const ( + detachedProcess = 0x00000008 + createNewProcessGroup = 0x00000200 + ) + cmd.SysProcAttr = &syscall.SysProcAttr{ + CreationFlags: detachedProcess | createNewProcessGroup, + } +} diff --git a/relaunch.go b/relaunch.go index a51910f..0b375d2 100644 --- a/relaunch.go +++ b/relaunch.go @@ -27,11 +27,23 @@ import ( // other caller still belongs. Two self-relaunches then differed by that one // line, and only the hidden one was ever reported broken. // -// So: no SysProcAttr at all, which is what RestartApp already did and why the -// database-switch relaunch never showed the fault. There is no console to -// suppress either way — OpsLog is linked for the Windows GUI subsystem. +// So: nothing that touches the WINDOW. RestartApp never called hideConsole and +// the database-switch relaunch never showed the fault, which is what proved it. +// There is no console to suppress either way — OpsLog is linked for the Windows +// GUI subsystem. What SysProcAttr does carry now is detachment, below, which is +// about process parentage and not about the window. +// Two things the PowerShell helper did that the direct launch did not, both +// restored here after operators kept reporting the relaunch failing: +// +// 1. It launched through Start-Process, so the new OpsLog was a child of +// PowerShell — which then exited. The direct launch makes it a child of +// the OpsLog that is dying, inside the same process group and console. +// detachProcess puts it back on its own. +// 2. It slept 400 ms after the old process was gone, before starting +// anything. See postExitSettle in main.go. func relaunchCmd(exe string, args ...string) *exec.Cmd { cmd := exec.Command(exe, args...) cmd.Dir = filepath.Dir(exe) + detachProcess(cmd) return cmd } diff --git a/relaunch_test.go b/relaunch_test.go index 5a10b54..7144f1c 100644 --- a/relaunch_test.go +++ b/relaunch_test.go @@ -17,13 +17,10 @@ import ( // load": a process in the task manager, no window, and killing it then starting // it by hand working every time. // -// Nil, not "some specific value": there is nothing a self-relaunch needs from -// STARTUPINFO, and anything set there is a window flag waiting to be wrong. -func TestRelaunchCmdDoesNotTouchTheWindow(t *testing.T) { +// The window flags themselves are asserted in relaunch_windows_test.go, where +// SysProcAttr has the fields to look at. +func TestRelaunchCmdPassesItsArgumentsAndFolder(t *testing.T) { cmd := relaunchCmd(filepath.Join("C:", "OpsLog", "OpsLog.exe"), "--post-update", "--wait-pid", "1234") - if cmd.SysProcAttr != nil { - t.Errorf("relaunchCmd set SysProcAttr = %+v; a self-relaunch must leave the window alone", cmd.SysProcAttr) - } if len(cmd.Args) != 4 || cmd.Args[1] != "--post-update" || cmd.Args[3] != "1234" { t.Errorf("args = %v, want the exe plus the three passed through", cmd.Args) } diff --git a/relaunch_windows_test.go b/relaunch_windows_test.go new file mode 100644 index 0000000..f543738 --- /dev/null +++ b/relaunch_windows_test.go @@ -0,0 +1,50 @@ +//go:build windows + +package main + +import ( + "path/filepath" + "testing" +) + +// A relaunch of OpsLog itself must not suppress the new process's window. +// +// HideWindow becomes SW_HIDE in the STARTUPINFO, and Windows applies it to the +// first top-level window the new process shows. That is how the updated OpsLog +// came to start perfectly — mutex taken, rig connected — and stay invisible: +// two operators on 0.27.23 reported a process in the task manager, no window, +// and killing it then starting OpsLog by hand working every time. +// +// CREATE_NO_WINDOW is refused for the same reason it is pointless: there is no +// console to suppress on a GUI-subsystem binary, and it is one flag away from +// the one that broke this. +func TestRelaunchNeverHidesTheWindow(t *testing.T) { + cmd := relaunchCmd(filepath.Join("C:", "OpsLog", "OpsLog.exe"), "--post-update") + if cmd.SysProcAttr == nil { + t.Fatal("no SysProcAttr — the relaunch should be detached (see detachProcess)") + } + if cmd.SysProcAttr.HideWindow { + t.Error("HideWindow is set: the relaunched OpsLog would start with no window") + } + const createNoWindow = 0x08000000 + if cmd.SysProcAttr.CreationFlags&createNoWindow != 0 { + t.Error("CREATE_NO_WINDOW is set on a GUI-subsystem relaunch") + } +} + +// Detached and in its own process group, which is what Start-Process gave the +// relaunch before the PowerShell helper was removed. Being a child of the +// instance that is dying is the one difference from the code that worked. +func TestRelaunchIsDetached(t *testing.T) { + cmd := relaunchCmd(filepath.Join("C:", "OpsLog", "OpsLog.exe"), "--post-update") + const ( + detachedProcess = 0x00000008 + createNewProcessGroup = 0x00000200 + ) + if cmd.SysProcAttr.CreationFlags&detachedProcess == 0 { + t.Error("DETACHED_PROCESS is missing: the new instance keeps the old one's console") + } + if cmd.SysProcAttr.CreationFlags&createNewProcessGroup == 0 { + t.Error("CREATE_NEW_PROCESS_GROUP is missing: a cleanup aimed at the old instance can reach the new one") + } +} diff --git a/update.go b/update.go index 14ecd9b..51974a1 100644 --- a/update.go +++ b/update.go @@ -243,8 +243,15 @@ func (a *App) DownloadAndApplyUpdate(url string) error { // differing by one line is how only one of them was broken. cmd := relaunchCmd(exe, "--post-update", "--wait-pid", strconv.Itoa(os.Getpid())) if err := cmd.Start(); err != nil { + applog.Printf("update: the relaunch could not be started: %v", err) return fmt.Errorf("schedule relaunch: %w", err) } + // The child's pid, named in BOTH logs — this one and the new instance's + // startup.log, which records the pid it was told to wait for. Without the + // pair there is no way to tell "the new instance never started" from "it + // started and gave up waiting", and those have different causes. + applog.Printf("update: relaunch started as pid %d, waiting for this one (pid %d) to exit", + cmd.Process.Pid, os.Getpid()) // Released rather than waited on: this process is about to exit, and a child // that outlives its parent must not be left as a zombie handle. _ = cmd.Process.Release()