From c12d1415dc869358ebb9180268161c84de4cbe5b Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 23 Oct 2019 14:35:53 +0100 Subject: [PATCH 1/3] Only attempt to kill parent once --- modules/graceful/restart.go | 13 +++++++++++++ modules/graceful/server.go | 8 ++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/modules/graceful/restart.go b/modules/graceful/restart.go index 5cba0581a56c7..7a6cf9862b6fb 100644 --- a/modules/graceful/restart.go +++ b/modules/graceful/restart.go @@ -12,8 +12,21 @@ 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 { + _ = syscall.Kill(syscall.Getppid(), 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 abe1b3d6d0887..16e2ea63dab4c 100644 --- a/modules/graceful/server.go +++ b/modules/graceful/server.go @@ -110,9 +110,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 +154,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) From 5a24ff079d97a38ef1555bf8cb2881609d9e9c32 Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 23 Oct 2019 14:53:07 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com> --- modules/graceful/restart.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/graceful/restart.go b/modules/graceful/restart.go index 7a6cf9862b6fb..04ee072c80ee6 100644 --- a/modules/graceful/restart.go +++ b/modules/graceful/restart.go @@ -22,7 +22,10 @@ var killParent sync.Once func KillParent() { killParent.Do(func() { if IsChild { - _ = syscall.Kill(syscall.Getppid(), syscall.SIGTERM) + ppid := syscall.Getppid() + if ppid > 1 { + _ = syscall.Kill(ppid, syscall.SIGTERM) + } } }) } From 8cbd9501d87bb7f34318f061539fc15a9ab9970e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Wed, 23 Oct 2019 15:53:34 +0100 Subject: [PATCH 3/3] Add waitgroup for running servers --- cmd/web.go | 2 ++ modules/graceful/graceful_windows.go | 5 +++++ modules/graceful/server.go | 9 +++++++++ modules/graceful/server_signals.go | 2 +- 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/web.go b/cmd/web.go index ae05b9e1452ee..3ca4041a7de34 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 753db87133a50..281b255fb5c58 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/server.go b/modules/graceful/server.go index 16e2ea63dab4c..896d547b46ca8 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() @@ -171,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 d0013b77af052..a4bcd00b16576 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 {