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

diff --git a/cmd/web.go b/cmd/web.go
index ae05b9e1452ee73c57dae5c8f47d0c0b7de80389..3ca4041a7de340abe0367996fda292b90c3a129b 100644 (file)
--- a/cmd/web.go
+++ b/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
 }

diff --git a/modules/graceful/graceful_windows.go b/modules/graceful/graceful_windows.go
index 753db87133a50e32ce08cc97122c01fee2630371..281b255fb5c58dfbf6ccc1ea473ab7f11ad1218a 100644 (file)
--- a/modules/graceful/graceful_windows.go
+++ b/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() {
+
+}

diff --git a/modules/graceful/restart.go b/modules/graceful/restart.go
index 5cba0581a56c7ad54e1b45f67157b34dbe39e757..04ee072c80ee6287a704e3ae18dd33987583c925 100644 (file)
--- a/modules/graceful/restart.go
+++ b/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

diff --git a/modules/graceful/server.go b/modules/graceful/server.go
index abe1b3d6d08875167a0f92b1a0b7af84c2bb6298..896d547b46ca838e5a9849107c567ac4fc392502 100644 (file)
--- a/modules/graceful/server.go
+++ b/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 {
-               _ = syscall.Kill(syscall.Getppid(), syscall.SIGTERM)
-       }
+       KillParent()
 
        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 {
-               _ = syscall.Kill(syscall.Getppid(), syscall.SIGTERM)
-       }
+       KillParent()
        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

diff --git a/modules/graceful/server_signals.go b/modules/graceful/server_signals.go
index d0013b77af052135be7a8d50084aab2c3d5c5849..a4bcd00b16576d0fbcd4c92146073ec268fcacd5 100644 (file)
--- a/modules/graceful/server_signals.go
+++ b/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 {