admin: Sync server variables (fix #4260) (#4274)

* Synchronize server assignment/references to avoid data race

* only hold lock during var reassignment
This commit is contained in:
Steven Angles 2021-08-16 17:04:47 -04:00 committed by GitHub
parent ab32440b21
commit a10910f398
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 49 additions and 20 deletions

View File

@ -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
)

View File

@ -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)
}
}