gitbayd: header and idle timeouts, and a drain on SIGTERM !193

merged merged by cmc on 2026-09-03 17:10 UTC · krz/gitbay:serve-shutdown into main

3 files changed, +106 −4

Layout: unified · split

cmd/gitbayd/main.go +47 −3

 	"net"
 	"net/http"
 	"os"
+	"os/signal"
 	"path/filepath"
 	"strconv"
 	"strings"
+	"syscall"
 	"time"
 	"github.com/spf13/cobra"
 			}
 			defer stopHookd()
+			// SIGTERM is how a deploy restarts the daemon: stop accepting,
+			// let what is in flight finish, exit 0 (#105).
+			ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
+			defer stop()
 			// Outbound webhook deliveries. The retry base is overridable
 			// for tests via GITBAY_WEBHOOK_RETRY_BASE.
 			retryBase := 30 * time.Second
 			}, buildinfo.String()).Run(whCtx)
 			errCh := make(chan error, 3)
+			var sshSrv *sshd.Server
+			var sshLn, gitLn net.Listener
 			if cfg.SSH.Mode == "embedded" {
 				srv, err := sshd.New(cfg, st)
 				if err != nil {
 					return err
 				}
 				slog.Info("ssh listening", "addr", ln.Addr())
+				sshSrv, sshLn = srv, ln
 				go func() { errCh <- srv.Serve(ln) }()
 			} else {
 				// system mode: the host sshd owns the SSH port and invokes
 			}
 			web := httpd.New(cfg, st)
-			hs := &http.Server{Addr: cfg.HTTP.Addr, Handler: web.Handler()}
+			// Header and idle timeouts bound what an idle or slow client can
+			// hold open. No write timeout: archives and upload-pack stream
+			// for as long as they take (#104).
+			hs := &http.Server{
+				Addr:              cfg.HTTP.Addr,
+				Handler:           web.Handler(),
+				ReadHeaderTimeout: 10 * time.Second,
+				IdleTimeout:       2 * time.Minute,
+				MaxHeaderBytes:    64 << 10,
+			}
 			go func() {
 				slog.Info("http listening", "addr", cfg.HTTP.Addr, "tls", cfg.HTTP.TLS)
 				switch cfg.HTTP.TLS {
 						})
 						go func() {
 							slog.Info("acme http listening", "addr", addr)
-							if err := http.ListenAndServe(addr, m.HTTPHandler(redirect)); err != nil {
+							acmeHTTP := &http.Server{Addr: addr, Handler: m.HTTPHandler(redirect),
+								ReadHeaderTimeout: 10 * time.Second, IdleTimeout: time.Minute}
+							if err := acmeHTTP.ListenAndServe(); err != nil {
 								slog.Warn("acme http listener failed; continuing with TLS-ALPN only", "err", err)
 							}
 						}()
 					return err
 				}
 				slog.Info("git-daemon listening", "addr", gln.Addr())
+				gitLn = gln
 				go func() { errCh <- gitd.New(cfg, st).Serve(gln) }()
 			}
-			return <-errCh
+			select {
+			case err := <-errCh:
+				return err
+			case <-ctx.Done():
+			}
+			slog.Info("shutting down")
+			stop()
+			for _, ln := range []net.Listener{sshLn, gitLn} {
+				if ln != nil {
+					ln.Close()
+				}
+			}
+			drain, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+			defer cancel()
+			if err := hs.Shutdown(drain); err != nil {
+				slog.Warn("http shutdown", "err", err)
+			}
+			if sshSrv != nil {
+				if err := sshSrv.Shutdown(drain); err != nil {
+					slog.Warn("ssh shutdown", "err", err)
+				}
+			}
+			return nil
 		},
 	}
 }

e2e/shutdown_test.go added +34

@@ -0,0 +1,34 @@
	1	package e2e
	2
	3	import (
	4	"net/http"
	5	"syscall"
	6	"testing"
	7	"time"
	8	)
	9
	10	// SIGTERM is how a deploy restarts the daemon. It used to be a plain
	11	// kill: no listener closed, no request or push allowed to finish. The
	12	// daemon now stops its listeners, drains, and exits 0 (#105).
	13	func TestServeStopsOnSIGTERM(t *testing.T) {
	14	inst := startInstance(t)
	15	if resp, err := http.Get(inst.base() + "/healthz"); err != nil \|\| resp.StatusCode != 200 {
	16	t.Fatalf("healthz before shutdown: %v", err)
	17	}
	18	if err := inst.proc.Process.Signal(syscall.SIGTERM); err != nil {
	19	t.Fatal(err)
	20	}
	21	done := make(chan error, 1)
	22	go func() { done <- inst.proc.Wait() }()
	23	select {
	24	case err := <-done:
	25	if err != nil {
	26	t.Fatalf("daemon did not exit cleanly on SIGTERM: %v", err)
	27	}
	28	case <-time.After(15 * time.Second):
	29	t.Fatal("daemon still running 15s after SIGTERM")
	30	}
	31	if _, err := http.Get(inst.base() + "/healthz"); err == nil {
	32	t.Fatal("http listener still answering after shutdown")
	33	}
	34	}

internal/sshd/sshd.go +25 −1

 package sshd
 import (
+	"context"
 	"crypto/ed25519"
 	"crypto/rand"
 	"encoding/base64"
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 	"golang.org/x/crypto/ssh"
 	st          *store.Store
 	sshCfg      *ssh.ServerConfig
 	authLimiter *rateLimiter
+	sessions    sync.WaitGroup // accepted connections still being served
 }
 func New(cfg config.Config, st *store.Store) (*Server, error) {
 		if err != nil {
 			return err
 		}
-		go s.handleConn(conn)
+		s.sessions.Add(1)
+		go func() {
+			defer s.sessions.Done()
+			s.handleConn(conn)
+		}()
+	}
+}
+// Shutdown waits for every accepted connection to finish, or for ctx. The
+// caller closes the listener first; a push in flight completes rather
+// than being cut mid-pack.
+func (s *Server) Shutdown(ctx context.Context) error {
+	done := make(chan struct{})
+	go func() {
+		s.sessions.Wait()
+		close(done)
+	}()
+	select {
+	case <-done:
+		return nil
+	case <-ctx.Done():
+		return ctx.Err()
 	}
 }