Skip to content

Commit

Permalink
Fix a goroutine leak in the signal handler code path
Browse files Browse the repository at this point in the history
The root cause of this issue was regarding our change to shutting down via
context. While implementating that change set there was a goroutine leak
introduced that did not provide a clean way for shutting down and reloading the
signal handler. This caused signal handling to be duplicated across reloads.

Fixes: #521
  • Loading branch information
Justin Reagor committed Oct 19, 2017
1 parent c91b379 commit bd75e61
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 13 deletions.
2 changes: 1 addition & 1 deletion core/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (a *App) Run() {
}()

a.Bus = events.NewEventBus()
a.handleSignals(cancel)
a.handleSignals(ctx, cancel)
a.ControlServer.Run(ctx, a.Bus)
a.runTasks(ctx, completedCh)

Expand Down
26 changes: 16 additions & 10 deletions core/signals.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,26 @@ import (
)

// HandleSignals listens for and captures signals used for orchestration
func (a *App) handleSignals(cancel context.CancelFunc) {
func (a *App) handleSignals(ctx context.Context, cancel context.CancelFunc) {
recvSig := make(chan os.Signal, 1)
signal.Notify(recvSig, syscall.SIGTERM, syscall.SIGINT)
go func() {
for sig := range recvSig {
switch sig {
case syscall.SIGINT:
a.Terminate()
break
case syscall.SIGTERM:
a.Terminate()
break
for {
select {
case sig := <-recvSig:
switch sig {
case syscall.SIGINT:
a.Terminate()
return
case syscall.SIGTERM:
a.Terminate()
return
default:
}
case <-ctx.Done():
return
default:
}
}
cancel()
}()
}
4 changes: 2 additions & 2 deletions core/signals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ func TestTerminateSignal(t *testing.T) {
// Test that only ensures that we cover a straight-line run through
// the handleSignals setup code
func TestSignalWiring(t *testing.T) {
_, cancel := context.WithCancel(context.Background())
ctx, cancel := context.WithCancel(context.Background())
app := EmptyApp()
app.Bus = events.NewEventBus()
app.handleSignals(cancel)
app.handleSignals(ctx, cancel)
sendAndWaitForSignal(t, syscall.SIGTERM)
}

Expand Down

0 comments on commit bd75e61

Please sign in to comment.