Skip to content

Commit

Permalink
internal/http: fix deadlock on Check.Shutdown
Browse files Browse the repository at this point in the history
  • Loading branch information
jroimartin committed Feb 4, 2025
1 parent 6b8c119 commit cd5a706
Showing 1 changed file with 25 additions and 21 deletions.
46 changes: 25 additions & 21 deletions internal/http/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ import (

// Check stores all the information needed to run a check locally.
type Check struct {
Logger *log.Entry
Name string
checker Checker
config *config.Config
port int
exitSignal chan os.Signal // used to stopt the server either by an os Signal or by calling Shutdown()
shutdownSignal chan bool // used to signal the server to shutdown.
finished chan error // used to wait for the server to shutted down.
Logger *log.Entry
Name string
checker Checker
config *config.Config
port int

exitSignal chan os.Signal // used to stop the server via OS signal.
shutdownSignal chan struct{} // used to stop the server via Check.Shutdown call.
serverErr chan error // used to handle server errors.
}

type Job struct {
Expand Down Expand Up @@ -165,47 +166,50 @@ func (c *Check) RunAndServe() {
}

c.Logger.Info(fmt.Sprintf("Listening at %s", server.Addr))
chanErr := make(chan error)
go func() {
if err := server.ListenAndServe(); !errors.Is(err, http.ErrServerClosed) {
chanErr <- err
c.serverErr <- err
} else {
c.finished <- nil
c.serverErr <- nil
}
close(c.serverErr)
}()

select {
case err := <-chanErr:
case err := <-c.serverErr:
// No need to shutdow the server because it was not started.
c.Logger.WithError(err).Error("ListenAndServe: Unable to start server")
return // No need to shutdow the server because it was not started.
return
case s := <-c.exitSignal:
c.Logger.WithField("signal", s.String()).Info("Signal received")
case <-c.shutdownSignal:
c.Logger.Info("Shutdown request received")
}

c.Logger.Info("Stopping server")

secs := 30
if c.config.ShutdownTimeout != nil {
secs = *c.config.ShutdownTimeout
}
ctx, cancel := context.WithTimeout(context.Background(), time.Second*time.Duration(secs))
defer cancel()

if err := server.Shutdown(ctx); err != nil {
// Some requests were canceled, but the server was shutdown.
c.Logger.WithError(err).Warn("Shutting down server")
}

c.Logger.Info("Finished RunAndServe")
}

// Shutdown is needed to fulfil the check interface and in this case we are
// shutting down the http server and waiting
// Shutdown is needed to fulfil the check interface and in this case
// we are shutting down the HTTP server and waiting.
func (c *Check) Shutdown() error {
// Send the exit signal to shutdown the server.
c.shutdownSignal <- true
// Signal the server to shutdown.
close(c.shutdownSignal)

c.Logger.Info("Shutdown: waiting for server shutdown")
return <-c.finished
return <-c.serverErr
}

// NewCheck creates new check to be run from the command line without having an agent.
Expand All @@ -215,9 +219,9 @@ func NewCheck(name string, checker Checker, logger *log.Entry, conf *config.Conf
Logger: logger,
config: conf,
exitSignal: make(chan os.Signal, 1),
shutdownSignal: make(chan bool),
shutdownSignal: make(chan struct{}),
port: *conf.Port,
finished: make(chan error),
serverErr: make(chan error),
}
signal.Notify(c.exitSignal, syscall.SIGINT, syscall.SIGTERM)
c.checker = checker
Expand Down

0 comments on commit cd5a706

Please sign in to comment.