From f067e12859715bcf33897ce2b7edc825708d0bae Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 23 Oct 2019 16:32:19 +0100 Subject: [PATCH] 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 | 2 ++ modules/graceful/graceful_windows.go | 5 +++++ modules/graceful/restart.go | 16 ++++++++++++++++ modules/graceful/server.go | 17 +++++++++++------ modules/graceful/server_signals.go | 2 +- 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/cmd/web.go b/cmd/web.go index ae05b9e145..3ca4041a7d 100644 --- 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 753db87133..281b255fb5 100644 --- 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 5cba0581a5..04ee072c80 100644 --- 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 abe1b3d6d0..896d547b46 100644 --- 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 d0013b77af..a4bcd00b16 100644 --- 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 {