From d0257cdefe72538f3d927e6738e829b454524fac Mon Sep 17 00:00:00 2001 From: Ben Brooks Date: Mon, 1 Dec 2025 18:27:53 +0000 Subject: [PATCH 1/3] Redo ServerContext lock changes --- rest/server_context.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/rest/server_context.go b/rest/server_context.go index 959ff4b75e..8e41ae789b 100644 --- a/rest/server_context.go +++ b/rest/server_context.go @@ -77,6 +77,7 @@ type ServerContext struct { cpuPprofFileMutex sync.Mutex // Protect cpuPprofFile from concurrent Start and Stop CPU profiling requests cpuPprofFile *os.File // An open file descriptor holds the reference during CPU profiling _httpServers map[serverType]*serverInfo // A list of HTTP servers running under the ServerContext + _httpServersLock sync.RWMutex // Lock for managing access to _httpServers GoCBAgent *gocbcore.Agent // GoCB Agent to use when obtaining management endpoints NoX509HTTPClient *http.Client // httpClient for the cluster that doesn't include x509 credentials, even if they are configured for the cluster hasStarted chan struct{} // A channel that is closed via PostStartup once the ServerContext has fully started @@ -224,15 +225,15 @@ func (sc *ServerContext) getServerAddr(s serverType) (string, error) { } func (sc *ServerContext) addHTTPServer(t serverType, s *serverInfo) { - sc.lock.Lock() - defer sc.lock.Unlock() + sc._httpServersLock.Lock() + defer sc._httpServersLock.Unlock() sc._httpServers[t] = s } // getHTTPServer returns information about the given HTTP server. func (sc *ServerContext) getHTTPServer(t serverType) (*serverInfo, error) { - sc.lock.RLock() - defer sc.lock.RUnlock() + sc._httpServersLock.RLock() + defer sc._httpServersLock.RUnlock() s, ok := sc._httpServers[t] if !ok { return nil, fmt.Errorf("server type %q not found running in server context", t) @@ -253,33 +254,38 @@ func (sc *ServerContext) PostStartup() { const serverContextStopMaxWait = 30 * time.Second func (sc *ServerContext) Close(ctx context.Context) { - - err := base.TerminateAndWaitForClose(sc.statsContext.terminator, sc.statsContext.doneChan, serverContextStopMaxWait) - if err != nil { - base.InfofCtx(ctx, base.KeyAll, "Couldn't stop stats logger: %v", err) - } + // stop HTTP servers - prevents any further requests from coming in before we continue with tearing down everything else + sc.stopHTTPServers(ctx) // stop the config polling - err = base.TerminateAndWaitForClose(sc.BootstrapContext.terminator, sc.BootstrapContext.doneChan, serverContextStopMaxWait) - if err != nil { + if err := base.TerminateAndWaitForClose(sc.BootstrapContext.terminator, sc.BootstrapContext.doneChan, serverContextStopMaxWait); err != nil { base.InfofCtx(ctx, base.KeyAll, "Couldn't stop background config update worker: %v", err) } - sc.lock.Lock() - defer sc.lock.Unlock() - // close cached bootstrap bucket connections if sc.BootstrapContext != nil && sc.BootstrapContext.Connection != nil { sc.BootstrapContext.Connection.Close() } + if agent := sc.GoCBAgent; agent != nil { + if err := agent.Close(); err != nil { + base.WarnfCtx(ctx, "Error closing agent connection: %v", err) + } + } + + sc.lock.Lock() + defer sc.lock.Unlock() for _, db := range sc.databases_ { db.Close(ctx) _ = db.EventMgr.RaiseDBStateChangeEvent(ctx, db.Name, "offline", "Database context closed", &sc.Config.API.AdminInterface) } sc.databases_ = nil sc.invalidDatabaseConfigTracking.dbNames = nil +} +func (sc *ServerContext) stopHTTPServers(ctx context.Context) { + sc._httpServersLock.Lock() + defer sc._httpServersLock.Unlock() for _, s := range sc._httpServers { if s.server != nil { base.InfofCtx(ctx, base.KeyHTTP, "Closing HTTP Server: %v", s.addr) @@ -289,12 +295,6 @@ func (sc *ServerContext) Close(ctx context.Context) { } } sc._httpServers = nil - - if agent := sc.GoCBAgent; agent != nil { - if err := agent.Close(); err != nil { - base.WarnfCtx(ctx, "Error closing agent connection: %v", err) - } - } } // GetDatabase attempts to return the DatabaseContext of the database. It will load the database if necessary. From 4119214c2a5cfdde24e84ec861257c880eea55e9 Mon Sep 17 00:00:00 2001 From: Ben Brooks Date: Mon, 1 Dec 2025 18:42:27 +0000 Subject: [PATCH 2/3] Restore accidental removal of stats context close --- rest/server_context.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rest/server_context.go b/rest/server_context.go index 8e41ae789b..8973d94e6b 100644 --- a/rest/server_context.go +++ b/rest/server_context.go @@ -273,6 +273,10 @@ func (sc *ServerContext) Close(ctx context.Context) { } } + if err := base.TerminateAndWaitForClose(sc.statsContext.terminator, sc.statsContext.doneChan, serverContextStopMaxWait); err != nil { + base.InfofCtx(ctx, base.KeyAll, "Couldn't stop stats logger: %v", err) + } + sc.lock.Lock() defer sc.lock.Unlock() for _, db := range sc.databases_ { From 124aff0544464a9a9a27f70d042d9d307e3947e8 Mon Sep 17 00:00:00 2001 From: Ben Brooks Date: Tue, 2 Dec 2025 12:43:24 +0000 Subject: [PATCH 3/3] Add missing lock rename in ServerContext.WaitForRESTAPIs --- rest/persistent_config_test.go | 1 - rest/server_context.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rest/persistent_config_test.go b/rest/persistent_config_test.go index d5a5a231d5..56d9879231 100644 --- a/rest/persistent_config_test.go +++ b/rest/persistent_config_test.go @@ -822,7 +822,6 @@ func TestPersistentConfigRegistryRollbackAfterCreateFailure(t *testing.T) { // 3. InsertConfig for a different db, with collection conflict with the new, failed update (should fail with conflict, but succeed after GetDatabaseConfigs runs) // 4. InsertConfig for a different db, with collection conflict with the previous version (should fail with conflict, and continue to fail after GetDatabaseConfigs runs) // 5. DeleteConfig for the same db name (triggers rollback, then successfully deletes) - func TestPersistentConfigRegistryRollbackAfterUpdateFailure(t *testing.T) { base.TestRequiresCollections(t) base.SetUpTestLogging(t, base.LevelInfo, base.KeyHTTP, base.KeyConfig) diff --git a/rest/server_context.go b/rest/server_context.go index 8973d94e6b..175678bdba 100644 --- a/rest/server_context.go +++ b/rest/server_context.go @@ -205,8 +205,8 @@ func (sc *ServerContext) WaitForRESTAPIs(ctx context.Context) error { ctx, cancelFn := context.WithTimeout(ctx, timeout) defer cancelFn() err, _ := base.RetryLoop(ctx, "Wait for REST APIs", func() (shouldRetry bool, err error, value interface{}) { - sc.lock.RLock() - defer sc.lock.RUnlock() + sc._httpServersLock.RLock() + defer sc._httpServersLock.RUnlock() if len(sc._httpServers) == len(allServers) { return false, nil, nil }