Skip to content

CBG-2413: remove memory leak in cbgt.Cfg implementations - #8584

Open
torcolvin wants to merge 2 commits into
mainfrom
CBG-2413
Open

CBG-2413: remove memory leak in cbgt.Cfg implementations#8584
torcolvin wants to merge 2 commits into
mainfrom
CBG-2413

Conversation

@torcolvin

@torcolvin torcolvin commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

CBG-2413: remove memory leak in cbgt.Cfg implementations

If there are pending events from FireEvent but there are no consumers, the FireEvent will hang when <-cbgt.CbgEvent is called.

  • Create a Stop method with a cancellable context so that any sends are cancelled
  • create CfgSGMem which contains a cbgt.CfgMem implementation in Sync Gateway by using a shim that supports either DataStore or a map. The CfgSGMem is the CfgMem implementation.

Pre-review checklist

  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Integration Tests

If there are pending events from FireEvent but there are no consumers, the FireEvent will hang when <-cbgt.CbgEvent is called.

- Create a Stop method with a cancellable context so that any sends are cancelled
- create CfgSGMem which contains a cbgt.CfgMem implementation in Sync Gateway by using a shim that supports either
  DataStore or a map. The CfgSGMem is the CfgMem implementation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@torcolvin
torcolvin requested review from a team and a lite review from Copilot August 10, 2026 19:44
@factory-droid

factory-droid Bot commented Aug 10, 2026

Copy link
Copy Markdown

Droid finished @torcolvin's task —— View job


Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a goroutine leak/hang scenario in cbgt.Cfg event delivery by making Sync Gateway’s CfgSG event sending cancellable and by replacing the CE cbgt.NewCfgMem usage with an in-repo, stoppable in-memory CfgSG backend.

Changes:

  • Introduces a cancellable context + Stop() on base.CfgSG to unblock pending FireEvent/Refresh sends.
  • Adds an in-memory cfgMemoryStorage backend and exposes it via base.NewCbgtCfgMem(ctx) as a CE replacement for cbgt.NewCfgMem.
  • Wires DatabaseContext and tests to use the new CfgSG-based cfg and stop it during shutdown.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
db/database.go Switches CfgSG to *base.CfgSG, uses new NewCbgtCfgMem(ctx), and stops it on shutdown.
base/sg_cluster_cfg.go Adds cancellable context + Stop(), introduces cfgMemoryStorage, and updates event delivery to be cancel-aware.
base/sg_cluster_cfg_test.go Adds coverage for Get/Set/Del semantics, event delivery, and Stop() unblocking behavior across backends.
base/heartbeat_test.go Updates tests to pass context into NewCbgtCfgMem(ctx).
base/dcp_test.go Updates tests to pass context into NewCbgtCfgMem(ctx).
base/cbgt.go Removes the old cbgt.NewCfgMem wrapper in favor of the new CfgSG-based implementation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread base/sg_cluster_cfg.go
Comment thread base/sg_cluster_cfg.go Outdated
Comment thread base/sg_cluster_cfg.go Outdated

@factory-droid factory-droid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One correctness issue to address: CfgSG.Stop currently does not stop cfgNodePoller because the poller is started with the non-cancellable ctx, so polling goroutines can survive database stop/offline. The other candidate items were test robustness suggestions rather than concrete defects.

@torcolvin torcolvin self-assigned this Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants