Graceful fixes (#8645)

* Only attempt to kill parent once

* Apply suggestions from code review

Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>

* Add waitgroup for running servers

cmd/web.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"strings"
 
+	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/routers"
@@ -226,6 +227,7 @@ func runWeb(ctx *cli.Context) error {
 		log.Critical("Failed to start server: %v", err)
 	}
 	log.Info("HTTP Listener: %s Closed", listenAddr)
+	graceful.WaitForServers()
 	log.Close()
 	return nil
 }

modules/graceful/graceful_windows.go

@@ -9,3 +9,8 @@ package graceful
 
 // This file contains shims for windows builds
 const IsChild = false
+
+// WaitForServers waits for all running servers to finish
+func WaitForServers() {
+
+}

modules/graceful/restart.go

@@ -12,8 +12,24 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"sync"
+	"syscall"
 )
 
+var killParent sync.Once
+
+// KillParent sends the kill signal to the parent process if we are a child
+func KillParent() {
+	killParent.Do(func() {
+		if IsChild {
+			ppid := syscall.Getppid()
+			if ppid > 1 {
+				_ = syscall.Kill(ppid, syscall.SIGTERM)
+			}
+		}
+	})
+}
+
 // RestartProcess starts a new process passing it the active listeners. It
 // doesn't fork, but starts a new process using the same environment and
 // arguments as when it was originally started. This allows for a newly

modules/graceful/server.go

@@ -31,6 +31,7 @@ const (
 var (
 	// RWMutex for when adding servers or shutting down
 	runningServerReg sync.RWMutex
+	runningServerWG  sync.WaitGroup
 	// ensure we only fork once
 	runningServersForked bool
 
@@ -47,6 +48,7 @@ var (
 
 func init() {
 	runningServerReg = sync.RWMutex{}
+	runningServerWG = sync.WaitGroup{}
 
 	DefaultMaxHeaderBytes = 0 // use http.DefaultMaxHeaderBytes - which currently is 1 << 20 (1MB)
 }
@@ -69,6 +71,11 @@ type Server struct {
 	OnShutdown      func()
 }
 
+// WaitForServers waits for all running servers to finish
+func WaitForServers() {
+	runningServerWG.Wait()
+}
+
 // NewServer creates a server on network at provided address
 func NewServer(network, address string) *Server {
 	runningServerReg.Lock()
@@ -110,9 +117,7 @@ func (srv *Server) ListenAndServe(serve ServeFunction) error {
 
 	srv.listener = newWrappedListener(l, srv)
 
-	if IsChild {
+	KillParent()
-		_ = syscall.Kill(syscall.Getppid(), syscall.SIGTERM)
-	}
 
 	srv.BeforeBegin(srv.network, srv.address)
 
@@ -156,9 +161,7 @@ func (srv *Server) ListenAndServeTLSConfig(tlsConfig *tls.Config, serve ServeFun
 	wl := newWrappedListener(l, srv)
 	srv.listener = tls.NewListener(wl, tlsConfig)
 
-	if IsChild {
+	KillParent()
-		_ = syscall.Kill(syscall.Getppid(), syscall.SIGTERM)
-	}
 	srv.BeforeBegin(srv.network, srv.address)
 
 	return srv.Serve(serve)
@@ -175,10 +178,12 @@ func (srv *Server) ListenAndServeTLSConfig(tlsConfig *tls.Config, serve ServeFun
 func (srv *Server) Serve(serve ServeFunction) error {
 	defer log.Debug("Serve() returning... (PID: %d)", syscall.Getpid())
 	srv.setState(stateRunning)
+	runningServerWG.Add(1)
 	err := serve(srv.listener)
 	log.Debug("Waiting for connections to finish... (PID: %d)", syscall.Getpid())
 	srv.wg.Wait()
 	srv.setState(stateTerminate)
+	runningServerWG.Done()
 	// use of closed means that the listeners are closed - i.e. we should be shutting down - return nil
 	if err != nil && strings.Contains(err.Error(), "use of closed") {
 		return nil

modules/graceful/server_signals.go

@@ -48,7 +48,7 @@ func (srv *Server) handleSignals() {
 			if setting.GracefulRestartable {
 				log.Info("PID: %d. Received SIGHUP. Forking...", pid)
 				err := srv.fork()
-				if err != nil {
+				if err != nil && err.Error() != "another process already forked. Ignoring this one" {
 					log.Error("Error whilst forking from PID: %d : %v", pid, err)
 				}
 			} else {