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) -}