diff --git a/cmd/svcinit/main.go b/cmd/svcinit/main.go index cc40ec1..faf40de 100644 --- a/cmd/svcinit/main.go +++ b/cmd/svcinit/main.go @@ -285,17 +285,16 @@ func main() { testCancel() - // TODO(zbarsky): what is the right behavior here when services are crashing in ibazel mode? - - // This is a brittle way of draining a channel in a nonblocking way, - // consider instead signalling cancellation of the services with a - // context, letting them close the channel, and using a waitgroup to - // wait for them to exit. + // This is a brittle way of draining a channel in a nonblocking way, + // consider instead signalling cancellation of the services with a + // context, letting them close the channel, and using a waitgroup to + // wait for them to exit. + // See: https://github.com/hermeticbuild/rules_itest/issues/72 Drain: for { select { - case <-servicesErrCh: - // nothing + case crashErr := <-servicesErrCh: + log.Printf("Discarding pending service error before reload: %v", crashErr) default: break Drain } diff --git a/runner/runner.go b/runner/runner.go index 5fd3cf7..d9f57c7 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -9,6 +9,7 @@ import ( "reflect" "runtime" "sync" + "syscall" "time" "rules_itest/logger" @@ -198,9 +199,20 @@ func (r *Runner) UpdateSpecs(serviceSpecs ServiceSpecs, ibazelCmd []byte) error } for _, label := range updateActions.toReloadLabels { - _, err := r.serviceInstances[label].stdin.Write(ibazelCmd) + old := r.serviceInstances[label] + _, err := old.stdin.Write(ibazelCmd) if err != nil { - return err + // Service likely crashed — fall back to a full restart. + log.Printf("%s hot-reload stdin write failed, falling back to restart: %v", colorize(old.VersionedServiceSpec), err) + old.stdin.Close() + if stopErr := old.StopWithSignal(syscall.SIGKILL); stopErr != nil { + log.Printf("%s stop during crash fallback failed: %v", colorize(old.VersionedServiceSpec), stopErr) + } + delete(r.serviceInstances, label) + r.serviceInstances[label], err = prepareServiceInstance(r.ctx, serviceSpecs[label]) + if err != nil { + return err + } } } diff --git a/runner/service_instance.go b/runner/service_instance.go index 99b8cf6..e645d96 100644 --- a/runner/service_instance.go +++ b/runner/service_instance.go @@ -249,8 +249,10 @@ func (s *ServiceInstance) StopWithSignal(signal syscall.Signal) error { s.killed = true }() - if err != nil { - return err + // If the process has already exited (raced between killGroup above and here), + // return nil — the killGroup call above already attempted group cleanup. + if s.isDone() { + return nil } if signal == syscall.SIGKILL {