Skip to content

Commit 15cb73d

Browse files
author
Kenta Hayashi
committed
windows: wait for shutdown teardown to finish before main returns
kardianos/service v1.2.1's windowsService.Run returns as soon as SCM STOP is processed, which lets main return and the Go runtime call ExitProcess before otelcol.Shutdown (running on a parallel goroutine) can complete. In the pristine change this rarely mattered because Shutdown is intrinsically sub-second, but a slow-shutdown plugin, a network flush, or any future teardown work could be truncated -- the final "Shutdown complete." log line and any pending flushes would be dropped without any indication. Add a done-channel signalled by (*program).run and waited on by main after s.Run() returns: - shutdown_signal.go: runCompleteChan (chan struct{}) plus sync.Once guard, runCompleteTimeout = 30s, signalRunComplete(), waitRunComplete(). - amazon-cloudwatch-agent.go: (*program).run defers signalRunComplete so it fires on every code path, including the Windows fallback where handleTerminatingSignal calls os.Exit. main's Windows service branch waits on runCompleteChan after s.Run returns. kardianos still reports SERVICE_STOPPED to SCM as soon as prg.Stop returns, so the OS shutdown of other services proceeds in parallel. Only this process is held back, and only until its own teardown finishes -- capped at runCompleteTimeout so a stuck teardown cannot indefinitely hold up the OS. Tests: three new unit tests (idempotent signal, wait-then-signal, wait-then-timeout). All 47 existing + new tests pass. Empirically validated on Windows Server 2022: with a 10-second sleep injected inside otelcol Service.Shutdown, the earlier change loses the "Shutdown complete." log line entirely (main returns before Shutdown finishes), whereas with this wait the line is preserved and the full teardown sequence completes. No 6008 / Kernel-Power 41; total OS shutdown +10s (matching the injected delay).
1 parent a6a1a93 commit 15cb73d

3 files changed

Lines changed: 100 additions & 0 deletions

File tree

cmd/amazon-cloudwatch-agent/amazon-cloudwatch-agent.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ func (p *program) Start(_ service.Service) error {
435435
return nil
436436
}
437437
func (p *program) run() {
438+
defer signalRunComplete() // unblock main()'s waitRunComplete on any return
438439
stop = make(chan struct{})
439440
reloadLoop(
440441
stop,
@@ -649,6 +650,9 @@ func main() {
649650
if err != nil {
650651
log.Println("E! " + err.Error())
651652
}
653+
// Wait for (*program).run to finish so otelcol.Shutdown can
654+
// complete before the Go runtime calls ExitProcess.
655+
waitRunComplete()
652656
}
653657
} else {
654658
stop = make(chan struct{})

cmd/amazon-cloudwatch-agent/shutdown_signal.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package main
55

66
import (
77
"log"
8+
"sync"
89
"sync/atomic"
910
"time"
1011
)
@@ -40,3 +41,32 @@ func handleTerminatingSignalDispatch(stopCh <-chan struct{}, timeout time.Durati
4041
terminatingSignalReceived.Store(true)
4142
}
4243
}
44+
45+
// runCompleteChan is closed by signalRunComplete when (*program).run
46+
// finishes. main() waits on this via waitRunComplete so otelcol.Shutdown
47+
// and any pending flushes can complete before the process exits.
48+
// Declared as var so tests can substitute a fresh channel.
49+
var runCompleteChan = make(chan struct{})
50+
51+
// runCompleteOnce guards close(runCompleteChan).
52+
var runCompleteOnce sync.Once
53+
54+
// runCompleteTimeout bounds waitRunComplete so a stuck teardown cannot
55+
// indefinitely hold up the OS.
56+
var runCompleteTimeout = 30 * time.Second
57+
58+
// signalRunComplete unblocks waitRunComplete. Safe to call multiple times.
59+
func signalRunComplete() {
60+
runCompleteOnce.Do(func() { close(runCompleteChan) })
61+
}
62+
63+
// waitRunComplete blocks until signalRunComplete or runCompleteTimeout,
64+
// whichever comes first. Logs a warning on timeout so the OS is not
65+
// held indefinitely.
66+
func waitRunComplete() {
67+
select {
68+
case <-runCompleteChan:
69+
case <-time.After(runCompleteTimeout):
70+
log.Printf("W! Windows service: waited %v for shutdown teardown to complete; exiting anyway", runCompleteTimeout)
71+
}
72+
}

cmd/amazon-cloudwatch-agent/shutdown_signal_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,69 @@ func TestDispatch_SCMAcceptedButTimeout_SetsFallbackFlag(t *testing.T) {
9595
t.Fatal("expected fallback flag set after SCM path timeout")
9696
}
9797
}
98+
99+
// ---- runComplete channel + wait -------------------------------------
100+
101+
// resetRunComplete restores a fresh channel + Once so tests can exercise
102+
// signalRunComplete and waitRunComplete in isolation.
103+
func resetRunComplete() {
104+
runCompleteChan = make(chan struct{})
105+
runCompleteOnce = sync.Once{}
106+
}
107+
108+
// signalRunComplete must be idempotent (close of a closed channel would
109+
// panic without sync.Once).
110+
func TestSignalRunComplete_Idempotent(t *testing.T) {
111+
t.Cleanup(resetRunComplete)
112+
resetRunComplete()
113+
114+
signalRunComplete()
115+
signalRunComplete() // must not panic
116+
117+
select {
118+
case <-runCompleteChan:
119+
default:
120+
t.Fatal("runCompleteChan not closed after signalRunComplete()")
121+
}
122+
}
123+
124+
// waitRunComplete must return promptly once signalRunComplete is called.
125+
func TestWaitRunComplete_ReturnsWhenSignaled(t *testing.T) {
126+
t.Cleanup(resetRunComplete)
127+
resetRunComplete()
128+
oldTimeout := runCompleteTimeout
129+
runCompleteTimeout = 500 * time.Millisecond
130+
t.Cleanup(func() { runCompleteTimeout = oldTimeout })
131+
132+
go func() {
133+
time.Sleep(10 * time.Millisecond)
134+
signalRunComplete()
135+
}()
136+
137+
start := time.Now()
138+
waitRunComplete()
139+
elapsed := time.Since(start)
140+
if elapsed > 200*time.Millisecond {
141+
t.Fatalf("waitRunComplete took %v; expected quick return after signal", elapsed)
142+
}
143+
}
144+
145+
// waitRunComplete must fall through after runCompleteTimeout when no
146+
// signal arrives, so the OS is not held indefinitely.
147+
func TestWaitRunComplete_TimesOut(t *testing.T) {
148+
t.Cleanup(resetRunComplete)
149+
resetRunComplete()
150+
oldTimeout := runCompleteTimeout
151+
runCompleteTimeout = 20 * time.Millisecond
152+
t.Cleanup(func() { runCompleteTimeout = oldTimeout })
153+
154+
start := time.Now()
155+
waitRunComplete()
156+
elapsed := time.Since(start)
157+
if elapsed < 15*time.Millisecond {
158+
t.Fatalf("waitRunComplete returned in %v; expected ~20ms timeout", elapsed)
159+
}
160+
if elapsed > 200*time.Millisecond {
161+
t.Fatalf("waitRunComplete took %v; expected ~20ms timeout", elapsed)
162+
}
163+
}

0 commit comments

Comments
 (0)