Skip to content

Commit 511a231

Browse files
committed
Fix shutdown waitgroup panic
1 parent c55a017 commit 511a231

File tree

3 files changed

+67
-36
lines changed

3 files changed

+67
-36
lines changed

cmd/web.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ func serveInstall(cmd *cli.Command) error {
156156
case <-graceful.GetManager().IsShutdown():
157157
<-graceful.GetManager().Done()
158158
log.Info("PID: %d Gitea Web Finished", os.Getpid())
159-
log.GetManager().Close()
160159
return err
161160
default:
162161
}
@@ -170,6 +169,16 @@ func serveInstalled(c *cli.Command) error {
170169

171170
showWebStartupMessage("Prepare to run web server")
172171

172+
// Check for shutdown signal during startup
173+
select {
174+
case <-graceful.GetManager().IsShutdown():
175+
log.Info("Shutdown signal received during startup, aborting server start")
176+
<-graceful.GetManager().Done()
177+
log.Info("PID: %d Gitea Web Finished", os.Getpid())
178+
return nil
179+
default:
180+
}
181+
173182
if setting.AppWorkPathMismatch {
174183
log.Error("WORK_PATH from config %q doesn't match other paths from environment variables or command arguments. "+
175184
"Only WORK_PATH in config should be set and used. Please make sure the path in config file is correct, "+
@@ -226,12 +235,21 @@ func serveInstalled(c *cli.Command) error {
226235

227236
gtprof.EnableBuiltinTracer(util.Iif(setting.IsProd, 2000*time.Millisecond, 100*time.Millisecond))
228237

238+
// Check for shutdown signal before starting web server
239+
select {
240+
case <-graceful.GetManager().IsShutdown():
241+
log.Info("Shutdown signal received before starting web server, aborting")
242+
<-graceful.GetManager().Done()
243+
log.Info("PID: %d Gitea Web Finished", os.Getpid())
244+
return nil
245+
default:
246+
}
247+
229248
// Set up Chi routes
230249
webRoutes := routers.NormalRoutes()
231250
err := listen(webRoutes, true)
232251
<-graceful.GetManager().Done()
233252
log.Info("PID: %d Gitea Web Finished", os.Getpid())
234-
log.GetManager().Close()
235253
return err
236254
}
237255

modules/graceful/server.go

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ type Server struct {
3636
wg sync.WaitGroup
3737
state state
3838
lock *sync.RWMutex
39+
connections map[*wrappedConn]struct{}
40+
connectionsLock sync.RWMutex
3941
BeforeBegin func(network, address string)
4042
OnShutdown func()
4143
PerWriteTimeout time.Duration
@@ -53,6 +55,7 @@ func NewServer(network, address, name string) *Server {
5355
wg: sync.WaitGroup{},
5456
state: stateInit,
5557
lock: &sync.RWMutex{},
58+
connections: make(map[*wrappedConn]struct{}),
5659
network: network,
5760
address: address,
5861
PerWriteTimeout: setting.PerWriteTimeout,
@@ -178,6 +181,21 @@ func (srv *Server) setState(st state) {
178181
srv.state = st
179182
}
180183

184+
// closeAllConnections forcefully closes all active connections
185+
func (srv *Server) closeAllConnections() {
186+
srv.connectionsLock.Lock()
187+
connections := make([]*wrappedConn, 0, len(srv.connections))
188+
for conn := range srv.connections {
189+
connections = append(connections, conn)
190+
}
191+
srv.connectionsLock.Unlock()
192+
193+
// Close all connections outside the lock to avoid deadlock
194+
for _, conn := range connections {
195+
_ = conn.Conn.Close() // Force close the underlying connection
196+
}
197+
}
198+
181199
type filer interface {
182200
File() (*os.File, error)
183201
}
@@ -216,16 +234,20 @@ func (wl *wrappedListener) Accept() (net.Conn, error) {
216234

217235
closed := int32(0)
218236

219-
c = &wrappedConn{
220-
Conn: c,
221-
server: wl.server,
222-
closed: &closed,
223-
perWriteTimeout: wl.server.PerWriteTimeout,
224-
perWritePerKbTimeout: wl.server.PerWritePerKbTimeout,
237+
wc := &wrappedConn{
238+
Conn: c,
239+
server: wl.server,
240+
closed: &closed,
225241
}
226242

227243
wl.server.wg.Add(1)
228-
return c, nil
244+
245+
// Track the connection
246+
wl.server.connectionsLock.Lock()
247+
wl.server.connections[wc] = struct{}{}
248+
wl.server.connectionsLock.Unlock()
249+
250+
return wc, nil
229251
}
230252

231253
func (wl *wrappedListener) Close() error {
@@ -244,17 +266,15 @@ func (wl *wrappedListener) File() (*os.File, error) {
244266

245267
type wrappedConn struct {
246268
net.Conn
247-
server *Server
248-
closed *int32
249-
deadline time.Time
250-
perWriteTimeout time.Duration
251-
perWritePerKbTimeout time.Duration
269+
server *Server
270+
closed *int32
271+
deadline time.Time
252272
}
253273

254274
func (w *wrappedConn) Write(p []byte) (n int, err error) {
255-
if w.perWriteTimeout > 0 {
256-
minTimeout := time.Duration(len(p)/1024) * w.perWritePerKbTimeout
257-
minDeadline := time.Now().Add(minTimeout).Add(w.perWriteTimeout)
275+
if w.server.PerWriteTimeout > 0 {
276+
minTimeout := time.Duration(len(p)/1024) * w.server.PerWritePerKbTimeout
277+
minDeadline := time.Now().Add(minTimeout).Add(w.server.PerWriteTimeout)
258278

259279
w.deadline = w.deadline.Add(minTimeout)
260280
if minDeadline.After(w.deadline) {
@@ -278,6 +298,12 @@ func (w *wrappedConn) Close() error {
278298
}
279299
}
280300
}()
301+
302+
// Remove from tracked connections
303+
w.server.connectionsLock.Lock()
304+
delete(w.server.connections, w)
305+
w.server.connectionsLock.Unlock()
306+
281307
w.server.wg.Done()
282308
}
283309
return w.Conn.Close()

modules/graceful/server_hooks.go

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package graceful
55

66
import (
77
"os"
8-
"runtime"
98

109
"code.gitea.io/gitea/modules/log"
1110
)
@@ -48,26 +47,14 @@ func (srv *Server) doShutdown() {
4847
}
4948

5049
func (srv *Server) doHammer() {
51-
defer func() {
52-
// We call srv.wg.Done() until it panics.
53-
// This happens if we call Done() when the WaitGroup counter is already at 0
54-
// So if it panics -> we're done, Serve() will return and the
55-
// parent will goroutine will exit.
56-
if r := recover(); r != nil {
57-
log.Error("WaitGroup at 0: Error: %v", r)
58-
}
59-
}()
6050
if srv.getState() != stateShuttingDown {
6151
return
6252
}
63-
log.Warn("Forcefully shutting down parent")
64-
for {
65-
if srv.getState() == stateTerminate {
66-
break
67-
}
68-
srv.wg.Done()
53+
log.Warn("Forcefully closing all connections")
6954

70-
// Give other goroutines a chance to finish before we forcibly stop them.
71-
runtime.Gosched()
72-
}
55+
// Close all active connections. Each connection's Close() method
56+
// will call wg.Done() exactly once, maintaining WaitGroup integrity.
57+
// This will allow wg.Wait() in Serve() to complete, and Serve() will
58+
// then set the state to stateTerminate.
59+
srv.closeAllConnections()
7360
}

0 commit comments

Comments
 (0)