From cd5a7062cda97203119d998c355b58cc46c297d2 Mon Sep 17 00:00:00 2001
From: Roi Martin <jroi.martin@gmail.com>
Date: Tue, 4 Feb 2025 14:42:47 +0100
Subject: [PATCH] internal/http: fix deadlock on Check.Shutdown

---
 internal/http/check.go | 46 +++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/internal/http/check.go b/internal/http/check.go
index 4a854bc..8bfdac2 100644
--- a/internal/http/check.go
+++ b/internal/http/check.go
@@ -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 {
@@ -165,19 +166,20 @@ 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:
@@ -185,27 +187,29 @@ func (c *Check) RunAndServe() {
 	}
 
 	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.
@@ -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