fix(update): put back the two things the PowerShell helper did
Operators are still losing the relaunch, so I went and read the code
from before 0430aab — the commit that removed the helper — instead of
theorising again. It was:
Wait-Process -Id <pid>; Start-Sleep -Milliseconds 400;
Start-Process -FilePath <exe> -ArgumentList '--post-update'
Two things in there that the direct launch never had:
1. Start-Process made the new OpsLog a child of PowerShell, which then
exited — so it was detached. The direct launch makes it a child of
the instance that is dying, in the same process group and console.
DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP puts it back on its own,
so nothing aimed at the old process can reach the new one.
2. It slept 400 ms AFTER the old process was gone, before starting
anything. A process's handles are released by the kernel as it dies,
so the mutex is free the moment 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. This is
not a theory about which of those it was; it is the pause being put
back where it was.
Not restored: the PowerShell itself. Defender removed 0.27.14 from a
station as Trojan:Script/Wacatac.H!ml, and "Script/" was that helper —
an unsigned binary replacing itself and spawning a windowless script to
start another executable is, byte for byte, a dropper. Bringing it back
trades this fault for one that deletes the program.
Also: the relaunch now logs the child's pid, which the new instance's
startup.log already records on the other side. Without the pair there is
no telling "the new instance never started" from "it started and gave up
waiting", and those have different causes.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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) {}
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
+15
-3
@@ -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
|
||||
}
|
||||
|
||||
+3
-6
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user