From 7f9455628a53c20e33956c49d33ef0ab65f6e071 Mon Sep 17 00:00:00 2001 From: Harshal Patel <106813066+HarshalPatel1972@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:50:21 +0530 Subject: [PATCH] internal/oom: Fix memory leak by removing watcher from map on Stop The oomWatchers.Stop method stopped the watcher but never removed it from the watchers map. Because Add is the only writer and nothing ever deletes from the map, *watcher entries accumulated for the lifetime of the shim process. This resulted in an unbounded memory leak. Furthermore, this prevented the same container ID from ever being watched again if it was re-created, silently dropping OOM monitoring. This commit modifies Stop to delete(ows.watchers, cid), ensuring the watcher is released and the container ID is freed. Signed-off-by: Harshal Patel <106813066+HarshalPatel1972@users.noreply.github.com> --- internal/oom/watcher.go | 5 +++-- internal/oom/watcher_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/internal/oom/watcher.go b/internal/oom/watcher.go index 29b45ff0b1..5e1761f8e9 100644 --- a/internal/oom/watcher.go +++ b/internal/oom/watcher.go @@ -80,10 +80,11 @@ func (ows *oomWatchers) Add(cid string, pid int, fn EventFunc) (retErr error) { func (ows *oomWatchers) Stop(cid string) error { ows.mu.Lock() - w, exist := ows.watchers[cid] + w := ows.watchers[cid] + delete(ows.watchers, cid) ows.mu.Unlock() - if !exist { + if w == nil { return nil } return w.stop() diff --git a/internal/oom/watcher_test.go b/internal/oom/watcher_test.go index e5212f3f05..5c6774af2a 100644 --- a/internal/oom/watcher_test.go +++ b/internal/oom/watcher_test.go @@ -99,3 +99,35 @@ func skipIfBinaryUnavailable(t *testing.T, binaryName string) { func toPtr[T comparable](v T) *T { return &v } + +func TestWatcherStopRemovesFromMap(t *testing.T) { + testutil.RequiresRoot(t) + + skipIfCgroupUnavailable(t) + + group := fmt.Sprintf("/%s", t.Name()) + mgr, err := cgroupsv2.NewManager(defaultCgroup2Path, group, &cgroupsv2.Resources{}) + require.NoError(t, err) + + // A dummy process to get a valid pid and cgroup + cmd := exec.Command("sleep", "10000") + require.NoError(t, cmd.Start()) + defer func() { + cmd.Process.Kill() + cmd.Wait() + }() + + require.NoError(t, mgr.AddProc(uint64(cmd.Process.Pid))) + + watchers := New() + containerID := "test-stop-removes" + + fn := func(cid string) {} + + require.NoError(t, watchers.Add(containerID, cmd.Process.Pid, fn)) + require.NoError(t, watchers.Stop(containerID)) + + // Should be able to add again with the same containerID + require.NoError(t, watchers.Add(containerID, cmd.Process.Pid, fn)) + require.NoError(t, watchers.Stop(containerID)) +}