fix(update): wait for the old process, not for a fixed window
The relaunch after an update stopped working, and the regression is mine: removing the PowerShell helper — which is what Defender was reading as a dropper — also removed the wait it was doing. Nothing took over the job. The numbers made it certain rather than unlucky. The instance being replaced is allowed THIRTY seconds to shut down (armExitWatchdog forces it out at that point) because it closes a remote logbook, a CAT session and sometimes a backup. The new instance was patient with the single-instance mutex for TWENTY. On any station where shutting down ran past that, the new process gave up and exited in silence: no window after an update, and the previous OpsLog still in the task manager. Exactly the report. Both relaunch paths now pass --wait-pid, and the new process waits on that process's handle — a plain kernel wait, which ends the instant the old one ends, however long or short that is, and looks nothing like a script starting another program. The mutex retry stays as a backstop and goes to forty-five seconds, so it is longer than the wait it exists for rather than shorter. And when the old process really has not gone, the message says that instead of "OpsLog is already running" — after an update the operator did not start a second copy, and what they need to know is which one to close. A test keeps the two spawn sites honest: a relaunch added without --wait-pid is this bug again.
This commit is contained in:
@@ -2979,8 +2979,11 @@ func (a *App) RestartApp() error {
|
||||
return fmt.Errorf("locate executable: %w", err)
|
||||
}
|
||||
// --relaunch: the child waits for OUR mutex instead of declaring us a
|
||||
// duplicate — this instance is quitting, just not always fast enough.
|
||||
cmd := exec.Command(exe, "--relaunch")
|
||||
// duplicate — this instance is quitting, just not always fast enough. And
|
||||
// --wait-pid, so it waits for this process to actually END rather than for a
|
||||
// window of time to pass: shutting down closes a logbook and a CAT session,
|
||||
// and on a remote database that takes as long as it takes.
|
||||
cmd := exec.Command(exe, "--relaunch", "--wait-pid", strconv.Itoa(os.Getpid()))
|
||||
cmd.Dir = filepath.Dir(exe)
|
||||
if err := cmd.Start(); err != nil {
|
||||
return fmt.Errorf("relaunch OpsLog: %w", err)
|
||||
|
||||
+4
-2
@@ -8,7 +8,8 @@
|
||||
"Preferences no longer says the section name twice — the small line above each panel repeated the heading right under it, and the sidebar beside it already shows which section is open.",
|
||||
"The band matrix can open on the digital mode you actually work. An operator who only ever does FT8 was shown DIGI every time and had to click through to their own mode on every callsign; Settings → General now chooses which digital row the matrix starts on. The row still rotates when you click it, and DIGI — all of them together — stays the default.",
|
||||
"The MQTT chip is gone from the status bar. That is the name of a message protocol, not of anything an operator has. The state it carried — the openings feed up or down, and how many reports have arrived — is in the Chase New panel, which is the place that uses it.",
|
||||
"The callsign box no longer narrows when you close the padlock. Its row gains a date field for a manual entry, and a flex row makes room by shrinking its children — so the widest box, the one the eye is on while typing, was the one that visibly moved. The callsign and both report boxes are now a notch narrower and fixed there, whether the date is showing or not."
|
||||
"The callsign box no longer narrows when you close the padlock. Its row gains a date field for a manual entry, and a flex row makes room by shrinking its children — so the widest box, the one the eye is on while typing, was the one that visibly moved. The callsign and both report boxes are now a notch narrower and fixed there, whether the date is showing or not.",
|
||||
"After an update, OpsLog starts again. The fix that stopped Defender calling the updater a trojan removed the helper that waited for the old process to die, and nothing took over the job: the new instance was patient with the single-instance lock for twenty seconds while the old one is allowed thirty to shut down — closing a remote logbook, a CAT session, sometimes a backup. Where that ran long the new process gave up in silence, leaving no window and a leftover OpsLog in the task manager. It now waits for the previous process itself, ending the instant it does; and if it really has not gone, it says so instead of claiming OpsLog is already running."
|
||||
],
|
||||
"fr": [
|
||||
"Les dialogues dans lesquels on tape ne reposent plus sur un fond flouté. Un filtre de fond couvre toute la fenêtre et est recalculé chaque fois que quoi que ce soit au-dessus se repeint — et derrière ces dialogues il y a une application qui ne s'arrête jamais de bouger : le CAT qui interroge quatre fois par seconde, les spots qui arrivent, les vumètres qui balaient, les cartes qui se redessinent. Pire à un endroit : l'éditeur de cluster s'ouvre depuis les Préférences, donc son fond était un deuxième filtre plein écran empilé sur le premier. Les Préférences, l'éditeur de cluster, l'éditeur de QSO, l'édition groupée, les règles d'alerte et les définitions de diplômes assombrissent désormais le fond au lieu de le flouter ; tout le reste garde le flou.",
|
||||
@@ -16,7 +17,8 @@
|
||||
"Les Préférences ne disent plus deux fois le nom de la section — la petite ligne au-dessus de chaque panneau répétait le titre juste en dessous, et la barre latérale montre déjà laquelle est ouverte.",
|
||||
"La matrice peut s'ouvrir sur le mode numérique que vous travaillez vraiment. Celui qui ne fait que du FT8 voyait DIGI à chaque fois et devait cliquer jusqu'à son mode pour chaque indicatif ; Réglages → Général choisit désormais la ligne numérique sur laquelle la matrice démarre. La ligne continue de tourner au clic, et DIGI — tous ensemble — reste le défaut.",
|
||||
"La pastille MQTT disparaît de la barre d'état. C'est le nom d'un protocole de messages, pas de quelque chose que possède un opérateur. Ce qu'elle indiquait — le flux d'ouvertures actif ou non, et le nombre de reports arrivés — est dans le panneau Chasse au nouveau, à l'endroit qui s'en sert.",
|
||||
"Le champ indicatif ne rétrécit plus quand on ferme le cadenas. Sa ligne gagne un champ date pour une saisie manuelle, et une ligne flex fait de la place en rétrécissant ses enfants — donc le plus large, celui que l'œil suit pendant la frappe, était celui qui bougeait visiblement. L'indicatif et les deux champs de report sont désormais un cran plus étroits et fixes, que la date soit affichée ou non."
|
||||
"Le champ indicatif ne rétrécit plus quand on ferme le cadenas. Sa ligne gagne un champ date pour une saisie manuelle, et une ligne flex fait de la place en rétrécissant ses enfants — donc le plus large, celui que l'œil suit pendant la frappe, était celui qui bougeait visiblement. L'indicatif et les deux champs de report sont désormais un cran plus étroits et fixes, que la date soit affichée ou non.",
|
||||
"Après une mise à jour, OpsLog redémarre. Le correctif qui a fait cesser la détection en cheval de Troie a supprimé l'assistant qui attendait la mort de l'ancien processus, et rien n'a repris ce travail : la nouvelle instance patientait vingt secondes sur le verrou d'instance unique alors que l'ancienne dispose de trente pour se fermer — elle referme un journal distant, une session CAT, parfois une sauvegarde. Quand cela durait, le nouveau processus abandonnait en silence : pas de fenêtre, et un OpsLog restant dans le gestionnaire des tâches. Il attend désormais l'ancien processus lui-même, et repart à l'instant où celui-ci s'arrête ; et s'il n'est vraiment pas parti, il le dit au lieu d'annoncer qu'OpsLog tourne déjà."
|
||||
]
|
||||
},
|
||||
{
|
||||
|
||||
@@ -3,6 +3,7 @@ package main
|
||||
import (
|
||||
"embed"
|
||||
"os"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -58,7 +59,17 @@ func acquireInstance(wait bool) bool {
|
||||
if !wait {
|
||||
return false
|
||||
}
|
||||
deadline := time.Now().Add(20 * time.Second)
|
||||
// Forty-five seconds, not twenty.
|
||||
//
|
||||
// The instance we are waiting for is allowed THIRTY to shut down — see
|
||||
// armExitWatchdog, which force-exits it at that point — because it closes a
|
||||
// remote logbook, a CAT session and sometimes a backup on the way out. A
|
||||
// twenty-second patience was therefore shorter than the wait it existed for,
|
||||
// and on a station where shutdown ran long the new instance gave up while
|
||||
// the old one was still finishing: no new window after an update, and a
|
||||
// leftover OpsLog in the task manager. This is the backstop; --wait-pid
|
||||
// below is the real answer.
|
||||
deadline := time.Now().Add(45 * time.Second)
|
||||
for time.Now().Before(deadline) {
|
||||
time.Sleep(300 * time.Millisecond)
|
||||
if acquireSingleInstance() {
|
||||
@@ -68,6 +79,23 @@ func acquireInstance(wait bool) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
// waitPidArg reads "--wait-pid N": the process this one must outlive.
|
||||
func waitPidArg(args []string) int {
|
||||
for i, a := range args {
|
||||
if a == "--wait-pid" && i+1 < len(args) {
|
||||
if n, err := strconv.Atoi(args[i+1]); err == nil {
|
||||
return n
|
||||
}
|
||||
}
|
||||
if v, ok := strings.CutPrefix(a, "--wait-pid="); ok {
|
||||
if n, err := strconv.Atoi(v); err == nil {
|
||||
return n
|
||||
}
|
||||
}
|
||||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
// processStart is stamped on the very first instruction of main, before the
|
||||
// single-instance guard and before anything else runs.
|
||||
//
|
||||
@@ -90,6 +118,20 @@ func main() {
|
||||
// to free instead of bailing out. Then clear the old exe it left behind.
|
||||
bootLogLaunch()
|
||||
postUpdate := hasFlag(os.Args[1:], "--post-update")
|
||||
// The instance that started us is still shutting down. Wait for it to
|
||||
// actually END — a kernel wait on its handle, which finishes the instant it
|
||||
// does — rather than hoping the mutex frees inside a fixed window. This is
|
||||
// what the old PowerShell helper did, and losing it is what left an operator
|
||||
// with no window after an update and the previous OpsLog still in the task
|
||||
// manager.
|
||||
if pid := waitPidArg(os.Args[1:]); pid > 0 {
|
||||
bootLog("waiting for the previous instance (pid %d) to exit", pid)
|
||||
if waitForProcessExit(pid, 60*time.Second) {
|
||||
bootLog("the previous instance is gone")
|
||||
} else {
|
||||
bootLog("the previous instance (pid %d) is STILL running after 60s — trying anyway", pid)
|
||||
}
|
||||
}
|
||||
// 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
|
||||
// got "OpsLog is already running" for following instructions. Same patience
|
||||
@@ -100,7 +142,16 @@ func main() {
|
||||
// window, no data folder, no log, which is indistinguishable from a
|
||||
// program that died on its first instruction.
|
||||
bootLog("another instance already holds the single-instance mutex - exiting")
|
||||
if postUpdate {
|
||||
// After an update the ordinary message is a lie by omission: the
|
||||
// operator did not start a second copy, the update did, and what
|
||||
// they need to know is that the PREVIOUS version never finished
|
||||
// closing.
|
||||
fatalBox("OpsLog", "The previous version of OpsLog has not finished closing, so the updated one cannot start.\n\n"+
|
||||
"Close the leftover OpsLog.exe in the Task Manager, then start OpsLog again — the update is already installed.")
|
||||
} else {
|
||||
fatalBox("OpsLog", "OpsLog is already running.\n\nLook for its window, or for a leftover OpsLog.exe in the Task Manager, and close it before starting another.")
|
||||
}
|
||||
return
|
||||
}
|
||||
bootLog("single-instance mutex acquired")
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"os"
|
||||
"regexp"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestWaitPidArg(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
args []string
|
||||
want int
|
||||
}{
|
||||
{"after the flag", []string{"--post-update", "--wait-pid", "4321"}, 4321},
|
||||
{"joined with =", []string{"--wait-pid=4321"}, 4321},
|
||||
{"absent", []string{"--post-update"}, 0},
|
||||
{"flag with nothing after it", []string{"--wait-pid"}, 0},
|
||||
{"not a number", []string{"--wait-pid", "later"}, 0},
|
||||
} {
|
||||
if got := waitPidArg(tc.args); got != tc.want {
|
||||
t.Errorf("%s: got %d, wanted %d", tc.name, got, tc.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Every relaunch has to tell the new process which one to wait for.
|
||||
//
|
||||
// The auto-update relaunch lost that when its PowerShell helper was removed —
|
||||
// the helper had waited for the pid, and nothing took over the job — and an
|
||||
// operator was left with no window after an update and the previous OpsLog
|
||||
// still running. This keeps the two spawn sites honest: if a relaunch is added
|
||||
// without --wait-pid, it is the same bug again.
|
||||
func TestEveryRelaunchPassesItsPid(t *testing.T) {
|
||||
spawn := regexp.MustCompile(`exec\.Command\(exe, "--(post-update|relaunch)"[^)]*\)`)
|
||||
for _, file := range []string{"update.go", "app.go"} {
|
||||
src, err := os.ReadFile(file)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", file, err)
|
||||
}
|
||||
for _, call := range spawn.FindAllString(string(src), -1) {
|
||||
if !strings.Contains(call, "--wait-pid") {
|
||||
t.Errorf("%s: %s does not tell the new instance which process to wait for", file, call)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2,8 +2,15 @@
|
||||
|
||||
package main
|
||||
|
||||
import "time"
|
||||
|
||||
// acquireSingleInstance is a no-op off Windows (the guard uses a Windows named
|
||||
// mutex), and during Wails' binding generation (the `bindings` tag) — that step
|
||||
// runs this binary, and a real OpsLog already running would otherwise make it
|
||||
// exit before Wails could reflect the bindings.
|
||||
func acquireSingleInstance() bool { return true }
|
||||
|
||||
// waitForProcessExit has nothing to wait on off Windows: there is no
|
||||
// single-instance guard there either, so the relaunch never has to queue behind
|
||||
// the old process.
|
||||
func waitForProcessExit(pid int, timeout time.Duration) bool { return true }
|
||||
|
||||
@@ -10,6 +10,7 @@ package main
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"time"
|
||||
"unsafe"
|
||||
|
||||
"golang.org/x/sys/windows"
|
||||
@@ -68,3 +69,35 @@ func focusExistingWindow() {
|
||||
showWindow.Call(hwnd, swRestore)
|
||||
setForeground.Call(hwnd)
|
||||
}
|
||||
|
||||
// waitForProcessExit blocks until the process with this pid is gone, or the
|
||||
// timeout runs out. Reports whether it actually went.
|
||||
//
|
||||
// This is what the auto-update relaunch needs, and it is the piece that was
|
||||
// lost when the PowerShell helper went. The old instance is allowed thirty
|
||||
// seconds to shut down (see armExitWatchdog) — it closes a remote logbook, a
|
||||
// CAT session, sometimes a backup — while the new one was only patient with the
|
||||
// mutex for twenty. On a station where shutdown took longer than that, the new
|
||||
// process gave up and exited, and the operator was left with the old one still
|
||||
// running and no new window: exactly the report.
|
||||
//
|
||||
// Waiting on a handle rather than sleeping a fixed time is also the honest
|
||||
// version: it ends the instant the old process ends, however long or short that
|
||||
// is, and it is a plain kernel wait — nothing that looks like a script starting
|
||||
// another program.
|
||||
func waitForProcessExit(pid int, timeout time.Duration) bool {
|
||||
if pid <= 0 {
|
||||
return true
|
||||
}
|
||||
h, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid))
|
||||
if err != nil {
|
||||
// Already gone, or not ours to wait on. Either way there is nothing to
|
||||
// wait for — and refusing to launch over an unexpected permission error
|
||||
// would be worse than starting.
|
||||
return true
|
||||
}
|
||||
defer windows.CloseHandle(h)
|
||||
ms := uint32(timeout / time.Millisecond)
|
||||
ev, err := windows.WaitForSingleObject(h, ms)
|
||||
return err == nil && ev == uint32(windows.WAIT_OBJECT_0)
|
||||
}
|
||||
|
||||
@@ -222,10 +222,18 @@ func (a *App) DownloadAndApplyUpdate(url string) error {
|
||||
// not our intentions, and an operator updating to 0.27.14 had OpsLog removed
|
||||
// under Trojan:Script/Wacatac.H!ml — the "Script/" being the PowerShell.
|
||||
//
|
||||
// The wait it existed for is not needed: --post-update already makes the new
|
||||
// instance patient with the single-instance mutex (see acquireInstance), so it
|
||||
// can start while this one is still shutting down and simply wait its turn.
|
||||
cmd := exec.Command(exe, "--post-update")
|
||||
// The wait it existed for still has to happen — it just happens on the other
|
||||
// side now. The new instance is told OUR pid and waits for this process to
|
||||
// end before taking the single-instance mutex.
|
||||
//
|
||||
// Waiting on the mutex alone was not enough, and that is the bug this line
|
||||
// fixes: shutting down is allowed thirty seconds here (armExitWatchdog),
|
||||
// because it closes a remote logbook, a CAT session and sometimes a backup,
|
||||
// while the new instance was only patient for twenty. On a station where
|
||||
// that ran long, the new process gave up and exited — leaving the old one
|
||||
// still running and no new window, which is precisely what the PowerShell
|
||||
// helper never did: it waited for the pid, however long it took.
|
||||
cmd := exec.Command(exe, "--post-update", "--wait-pid", strconv.Itoa(os.Getpid()))
|
||||
cmd.Dir = dir
|
||||
cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true, CreationFlags: 0x08000000} // CREATE_NO_WINDOW
|
||||
if err := cmd.Start(); err != nil {
|
||||
|
||||
Reference in New Issue
Block a user