From 494d0787e272f1c5f558dc2a2f3be6d962d8f635 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 10:43:22 +0200 Subject: [PATCH] Wait for a stopped bundle's exit instead of sleeping after it Launched.Stop now returns once the bundle's exit is handled, bounded by StopWait, and the launcher reads its restart pauses once at Start, so the exit goroutine no longer races a test that changes them. The caller tests drop their half-second pause (novox/hq issue 476). --- node-tools/internal/launch/launch.go | 36 ++++++++++++++++------ node-tools/internal/runtime/caller_test.go | 12 ++------ 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/node-tools/internal/launch/launch.go b/node-tools/internal/launch/launch.go index ce78b49..6e96aea 100644 --- a/node-tools/internal/launch/launch.go +++ b/node-tools/internal/launch/launch.go @@ -107,6 +107,10 @@ var ( RestartSettle = time.Minute ) +// StopWait bounds how long Launched.Stop waits for its bundle's exit to be handled; a bundle that +// ignores SIGTERM longer than this is left to exit on its own (novox/hq issue 476). +var StopWait = 5 * time.Second + // Launched is a running bundle: what it registered, and how to stop it. type Launched struct { Registrations []Registration @@ -143,6 +147,9 @@ type child struct { pending map[int64]chan message next int64 dead chan struct{} + // handled is closed once the child's exit has been handled whole — its state cleared and its + // restart decided — which Stop waits for (novox/hq issue 476). + handled chan struct{} why error // watches are the state watches this child asked for, stopped when it exits (ADR 0201): a child // that starts again watches again, as its code runs again. @@ -195,7 +202,10 @@ func Start(module, entry string, env []string, mesh Bus, logf func(string, ...an } var restart func(after time.Duration) var startedAt time.Time - pause := RestartFirst + // The restart pauses are read once, here: the exit handling runs on its own goroutine and must not + // read the package's settings while a caller (a test) changes them (novox/hq issue 476). + pauseFirst, pauseMost, settle := RestartFirst, RestartMost, RestartSettle + pause := pauseFirst start := func() (*child, error) { cmd := exec.Command(entry) @@ -215,7 +225,8 @@ func Start(module, entry string, env []string, mesh Bus, logf func(string, ...an if err := cmd.Start(); err != nil { return nil, err } - c := &child{cmd: cmd, stdin: stdin, pending: map[int64]chan message{}, next: 1, dead: make(chan struct{})} + c := &child{cmd: cmd, stdin: stdin, pending: map[int64]chan message{}, next: 1, dead: make(chan struct{}), + handled: make(chan struct{})} var lastSaid string var saidMu sync.Mutex stderrDone := make(chan struct{}) @@ -237,6 +248,7 @@ func Start(module, entry string, env []string, mesh Bus, logf func(string, ...an } }() go func() { + defer close(c.handled) scan := bufio.NewScanner(stdout) scan.Buffer(make([]byte, 64*1024), 16<<20) for scan.Scan() { @@ -387,14 +399,14 @@ func Start(module, entry string, env []string, mesh Bus, logf func(string, ...an } wasStopped := stopped // Started again at once if it ran a while; after a growing pause while it keeps exiting. - wait := RestartFirst - if time.Since(startedAt) < RestartSettle { + wait := pauseFirst + if time.Since(startedAt) < settle { wait = pause - if pause *= 2; pause > RestartMost { - pause = RestartMost + if pause *= 2; pause > pauseMost { + pause = pauseMost } } else { - pause = RestartFirst + pause = pauseFirst } mu.Unlock() if !wasStopped && wasServing { @@ -451,8 +463,8 @@ func Start(module, entry string, env []string, mesh Bus, logf func(string, ...an if err != nil { if !stopped { wait := pause - if pause *= 2; pause > RestartMost { - pause = RestartMost + if pause *= 2; pause > pauseMost { + pause = pauseMost } logf("[mesh-tools] %s's bundle did not start again: %v; trying in %s", module, err, wait) go restart(wait) @@ -557,6 +569,12 @@ func Start(module, entry string, env []string, mesh Bus, logf func(string, ...an mu.Unlock() if c != nil && c.cmd.Process != nil { _ = c.cmd.Process.Signal(syscall.SIGTERM) + // Returns once the bundle's exit is handled, so nothing of it runs on after Stop + // (novox/hq issue 476); bounded, for a bundle that does not exit on SIGTERM. + select { + case <-c.handled: + case <-time.After(StopWait): + } } }} for _, under := range order { diff --git a/node-tools/internal/runtime/caller_test.go b/node-tools/internal/runtime/caller_test.go index 54ca2fe..8228231 100644 --- a/node-tools/internal/runtime/caller_test.go +++ b/node-tools/internal/runtime/caller_test.go @@ -27,7 +27,7 @@ func TestACallOnTheCallerNamedSubjectReachesTheModuleWithItsCaller(t *testing.T) if err != nil { t.Fatal(err) } - defer settled(stop) + defer stop() // returns once the bundle's exit is handled (novox/hq issue 476) asker, err := bus.Connect(bus.Credential{URL: mt.URL(t), Module: "node-tools", Node: "shanks", User: "shanks.node-tools", Password: "unused"}) if err != nil { @@ -65,7 +65,7 @@ func TestTheSubjectThatNamesNoCallerStillWorksAndNamesNone(t *testing.T) { if err != nil { t.Fatal(err) } - defer settled(stop) + defer stop() // returns once the bundle's exit is handled (novox/hq issue 476) old, err := nats.Connect(mt.URL(t)) if err != nil { t.Fatal(err) @@ -89,11 +89,3 @@ func TestTheSubjectThatNamesNoCallerStillWorksAndNamesNone(t *testing.T) { } } } - -// settled stops a runtime and waits for its bundle's exit to be handled: the launcher reads its restart pauses -// (launch.RestartFirst) as a bundle exits, and a later test in this package sets them, so a test that leaves its -// bundle exiting behind it races the next one. -func settled(stop func()) { - stop() - time.Sleep(500 * time.Millisecond) -}