In version 0.0.52, I've noticed that StartAll() returns an error when service startup fails, and svcinit appears to panic on that error without calling StopAll().
I've got an itest_service Go runtime that starts a Docker container as a side effect. That Go wrapper is written to handle SIGTERM and clean up the container it created.
A second itest_service Go runtime depends on that Docker container side effect. When the second runtime fails its startup health check, StartAll() returns an error and svcinit exits via panic / must(err) without calling StopAll().
Because StopAll() never runs, the first runtime never receives SIGTERM, so it never gets a chance to clean up its Docker container. The side-effect container is left orphaned after the test run exits.
Expected Behavior
When StartAll() returns an error, svcinit should make a best-effort call to StopAll() before exiting nonzero.
The original startup error should still fail the test. Cleanup errors can be logged separately, but should not replace the original failure.
Proposed Change
Change the StartAll() error path from immediate must(err) to cleanup-before-exit behavior:
cmd/svcinit/main.go:169-175
criticalPath, err := r.StartAll(servicesErrCh)
-if errors.Is(err, context.Canceled) {
- _, err := r.StopAll()
- must(err)
- return
+if err != nil {
+ if errors.Is(err, context.Canceled) {
+ _, stopErr := r.StopAll()
+ must(stopErr)
+ return
+ }
+
+ _, stopErr := r.StopAll()
+ if stopErr != nil {
+ log.Printf("error while shutting down services: %v\n", stopErr)
+ }
+ log.Fatalf("failed to start services: %v\n", err)
}
-must(err)This would let already-started services receive shutdown and clean up their own side effects before svcinit exits.
In version
0.0.52, I've noticed thatStartAll()returns an error when service startup fails, andsvcinitappears to panic on that error without callingStopAll().I've got an
itest_serviceGo runtime that starts a Docker container as a side effect. That Go wrapper is written to handleSIGTERMand clean up the container it created.A second
itest_serviceGo runtime depends on that Docker container side effect. When the second runtime fails its startup health check,StartAll()returns an error andsvcinitexits viapanic/must(err)without callingStopAll().Because
StopAll()never runs, the first runtime never receivesSIGTERM, so it never gets a chance to clean up its Docker container. The side-effect container is left orphaned after the test run exits.Expected Behavior
When
StartAll()returns an error,svcinitshould make a best-effort call toStopAll()before exiting nonzero.The original startup error should still fail the test. Cleanup errors can be logged separately, but should not replace the original failure.
Proposed Change
Change the
StartAll()error path from immediatemust(err)to cleanup-before-exit behavior:cmd/svcinit/main.go:169-175
This would let already-started services receive shutdown and clean up their own side effects before
svcinitexits.