Skip to content

fix(rulemanager): scope monitorContainer exit to its own registration - #920

Merged
matthyx merged 1 commit into
mainfrom
fix/rulemanager-monitor-container-goroutine-leak-919
Aug 24, 2026
Merged

matthyx merged 1 commit into
mainfrom
fix/rulemanager-monitor-container-goroutine-leak-919

Conversation

@matthyx

@matthyx matthyx commented Aug 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

RuleManager.monitorContainer leaked one goroutine per container remove→add cycle that happened faster than its 5s polling ticker, because it re-derived its own liveness from shared mutable state instead of a registration-scoped signal. This gives each registration its own done channel so a goroutine can only be stopped by the removal that owns it.

Overview

RuleManager.monitorContainer used to re-derive its own liveness by polling rm.trackedContainers.Contains(k8sContainerID) on a 5s ticker instead of watching for its own registration's removal. If a container was removed and re-added faster than one tick, the stale goroutine's poll saw the ID "tracked again" (from the new Add) and looped forever, while ContainerCallback spawned a brand-new monitorContainer goroutine for the same ID — leaking one goroutine per such remove→add cycle, permanently, for the life of the process.

This gives each ContainerCallback EventTypeAddContainer its own scoped done chan struct{}, stored in a new RuleManager.trackedContainerDone map and passed directly into startRuleManager/monitorContainer as a parameter. monitorContainer now blocks on that channel (or rm.ctx.Done()) instead of polling shared state, and EventTypeRemoveContainer closes + clears the current registration's channel. Because the goroutine holds its channel via closure rather than re-reading a shared ID-keyed map at wake time, only the removal that owns a given registration can stop it — a sibling/later registration's channel has no effect on it. This closes the race structurally rather than narrowing the polling window, and also makes container-removal shutdown immediate instead of waiting up to 5s.

The now-unused syscallPeriod ticker and its container.Mntns == 0 debug check were removed along with the polling loop.

Additional Information

Added docs/features/rulemanager-monitor-container-lifecycle.md documenting the bug, the fix, and the observable behavior change.

How to Test

go test -race ./pkg/rulemanager/... -run 'TestMonitorContainer|TestContainerCallback' -v

New tests in pkg/rulemanager/containercallbacks_test.go:

  • TestMonitorContainer_ExitsOnDoneChannel — exits when its own channel closes.
  • TestMonitorContainer_IgnoresSiblingDoneChannel — does not exit when an unrelated channel closes.
  • TestContainerCallback_FastRemoveAddDoesNotLeakGoroutine — reproduces the Add→Remove→Add race end-to-end through ContainerCallback and asserts the stale registration's channel is closed while the current registration's channel stays open and is a distinct channel.

Related issues/PRs:

AI-skills: none | cmds: /oh-my-claudecode:autopilot

@matthyx matthyx added the ai-assisted Created through Armosec AI tooling (armosec-shared-rules plugin) label Aug 24, 2026
monitorContainer re-derived liveness by polling rm.trackedContainers on a
5s ticker instead of listening for its own registration's removal. A
container removed and re-added faster than one tick left the stale
goroutine's poll seeing the ID "tracked again" (from the new Add), so it
never exited while ContainerCallback spawned a second monitorContainer for
the same ID -- leaking one goroutine per such cycle.

Each ContainerCallback Add now creates a done channel scoped to that
registration and passes it directly into monitorContainer, which blocks on
it instead of polling shared state. Remove closes and clears the current
registration's channel, so only the removal that owns a given goroutine can
stop it -- a re-Add's fresh channel has no effect on the old one.

Fixes #919

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HS36ZHmKzkzibNi1oHVNUQ
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
@matthyx
matthyx force-pushed the fix/rulemanager-monitor-container-goroutine-leak-919 branch from e7d9fd2 to 6ab4261 Compare August 24, 2026 10:41
@coderabbitai

coderabbitai Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 57 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d10c2828-ddc1-4968-ac13-f79e65677309

📥 Commits

Reviewing files that changed from the base of the PR and between de4676b and 6ab4261.

📒 Files selected for processing (4)
  • docs/features/rulemanager-monitor-container-lifecycle.md
  • pkg/rulemanager/containercallbacks.go
  • pkg/rulemanager/containercallbacks_test.go
  • pkg/rulemanager/rule_manager.go
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/rulemanager-monitor-container-goroutine-leak-919

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Performance Benchmark Results

Node-Agent Resource Usage
Metric BEFORE AFTER Delta
Avg CPU (cores) 0.157 0.148 -5.7%
Peak CPU (cores) 0.164 0.154 -6.0%
Avg Memory (MiB) 333.147 272.233 -18.3%
Peak Memory (MiB) 335.398 278.984 -16.8%
Dedup Effectiveness

No data available.

@matthyx matthyx added the release Create release label Aug 24, 2026
@matthyx
matthyx merged commit 6534cfc into main Aug 24, 2026
40 checks passed
@matthyx
matthyx deleted the fix/rulemanager-monitor-container-goroutine-leak-919 branch August 24, 2026 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted Created through Armosec AI tooling (armosec-shared-rules plugin) release Create release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RuleManager.monitorContainer leaks a goroutine per container remove+add cycle faster than the 5s syscallTicker (TOCTOU on trackedContainers)

1 participant