diff --git a/admin.go b/admin.go index ff5bc713c..fb451682e 100644 --- a/admin.go +++ b/admin.go @@ -335,6 +335,7 @@ func replaceLocalAdminServer(cfg *Config) error { return err } + serverMu.Lock() localAdminServer = &http.Server{ Addr: addr.String(), // for logging purposes only Handler: handler, @@ -343,10 +344,14 @@ func replaceLocalAdminServer(cfg *Config) error { IdleTimeout: 60 * time.Second, MaxHeaderBytes: 1024 * 64, } + serverMu.Unlock() adminLogger := Log().Named("admin") go func() { - if err := localAdminServer.Serve(ln); !errors.Is(err, http.ErrServerClosed) { + serverMu.Lock() + server := localAdminServer + serverMu.Unlock() + if err := server.Serve(ln); !errors.Is(err, http.ErrServerClosed) { adminLogger.Error("admin server shutdown for unknown reason", zap.Error(err)) } }() @@ -474,6 +479,7 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error { return err } + serverMu.Lock() // create secure HTTP server remoteAdminServer = &http.Server{ Addr: addr.String(), // for logging purposes only @@ -485,6 +491,7 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error { MaxHeaderBytes: 1024 * 64, ErrorLog: serverLogger, } + serverMu.Unlock() // start listener ln, err := Listen(addr.Network, addr.JoinHostPort(0)) @@ -494,7 +501,10 @@ func replaceRemoteAdminServer(ctx Context, cfg *Config) error { ln = tls.NewListener(ln, tlsConfig) go func() { - if err := remoteAdminServer.Serve(ln); !errors.Is(err, http.ErrServerClosed) { + serverMu.Lock() + server := remoteAdminServer + serverMu.Unlock() + if err := server.Serve(ln); !errors.Is(err, http.ErrServerClosed) { remoteLogger.Error("admin remote server shutdown for unknown reason", zap.Error(err)) } }() @@ -1229,6 +1239,7 @@ var bufPool = sync.Pool{ // keep a reference to admin endpoint singletons while they're active var ( + serverMu sync.Mutex localAdminServer, remoteAdminServer *http.Server identityCertCache *certmagic.Cache ) diff --git a/admin_test.go b/admin_test.go index cfb4ab7bf..608a32c67 100644 --- a/admin_test.go +++ b/admin_test.go @@ -17,9 +17,28 @@ package caddy import ( "encoding/json" "reflect" + "sync" "testing" ) +var testCfg = []byte(`{ + "apps": { + "http": { + "servers": { + "myserver": { + "listen": ["tcp/localhost:8080-8084"], + "read_timeout": "30s" + }, + "yourserver": { + "listen": ["127.0.0.1:5000"], + "read_header_timeout": "15s" + } + } + } + } + } + `) + func TestUnsyncedConfigAccess(t *testing.T) { // each test is performed in sequence, so // each change builds on the previous ones; @@ -108,25 +127,24 @@ func TestUnsyncedConfigAccess(t *testing.T) { } } +// TestLoadConcurrent exercises Load under concurrent conditions +// and is most useful under test with `-race` enabled. +func TestLoadConcurrent(t *testing.T) { + var wg sync.WaitGroup + + for i := 0; i < 100; i++ { + wg.Add(1) + go func() { + _ = Load(testCfg, true) + wg.Done() + }() + } + + wg.Wait() +} + func BenchmarkLoad(b *testing.B) { for i := 0; i < b.N; i++ { - cfg := []byte(`{ - "apps": { - "http": { - "servers": { - "myserver": { - "listen": ["tcp/localhost:8080-8084"], - "read_timeout": "30s" - }, - "yourserver": { - "listen": ["127.0.0.1:5000"], - "read_header_timeout": "15s" - } - } - } - } - } - `) - Load(cfg, true) + Load(testCfg, true) } }