Add timeout to rungroup shutdown #1481
Conversation
| e := <-errors | ||
| level.Debug(g.logger).Log("msg", "successfully interrupted actor", "actor", e.errorSourceName, "index", i) | ||
| select { | ||
| case <-timeoutTimer.C: |
There was a problem hiding this comment.
I think this case potentially leaks goroutines, which might leave a zombie process? Might be better than hanging, might need a bigger hammer
There was a problem hiding this comment.
It will in the case of an autoupdate where we're reloading launcher, yeah -- that seemed preferable to hanging. The bigger hammer I can think of is just os.Exit and let launchctl/systemctl/service manager restart launcher, but I know we've been hesitant to do that in the past.
| @@ -65,15 +73,39 @@ func (g *Group) Run() error { | |||
| level.Debug(g.logger).Log("msg", "received interrupt error from first actor -- shutting down other actors", "err", initialActorErr) | |||
|
|
|||
| // Signal all actors to stop. | |||
| numActors := int64(len(g.actors)) | |||
| interruptWait := semaphore.NewWeighted(numActors) | |||
There was a problem hiding this comment.
semaphore.NewWeighted is interesting as compared to a waitgroup. I think it's good, though a little harder to think about all the edges.
Maybe cleaner than the channels I used for osquery/osquery-go#108, less sure about perf. But that's not a real concern here.
Ensure that the rungroup will still exit in a timely manner even if a rungroup actor a) doesn't interrupt in a timely manner or b) doesn't terminate its execution function in a timely manner. This prevents any rungroup actor from holding launcher in a suspended state, where it isn't doing anything but also isn't exiting in order to reload itself.
Relates to #1205