From dc64635ef02ffe6467ae347d1784963712ef4366 Mon Sep 17 00:00:00 2001 From: DeathGun44 Date: Fri, 23 Oct 2026 13:11:36 +0530 Subject: [PATCH] fix(processtree): walk container branches under the creator lock GetContainerProcessTree walked the process map under the manager lock only, while exit cleanup reparents children under the creator lock only. Route the walk through the creator's existing GetPidBranch so both sides take the same lock. Signed-off-by: DeathGun44 --- .../creator/processtree_creator.go | 3 +- .../creator/processtree_creator_interface.go | 2 + pkg/processtree/process_tree_manager.go | 2 +- pkg/processtree/process_tree_manager_test.go | 44 +++++++++++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/pkg/processtree/creator/processtree_creator.go b/pkg/processtree/creator/processtree_creator.go index 1d38ffc928..a4a08d5df3 100644 --- a/pkg/processtree/creator/processtree_creator.go +++ b/pkg/processtree/creator/processtree_creator.go @@ -148,7 +148,8 @@ func (pt *processTreeCreatorImpl) GetProcessBootTimeNs(pid uint32) uint64 { return pt.pidStartTimeNs[pid] } -// GetPidBranch performs container branch operation (no longer needs to be atomic) +// GetPidBranch walks the container branch under the creator's lock, so exit +// cleanup cannot reparent nodes mid-walk. func (pt *processTreeCreatorImpl) GetPidBranch(containerTree any, containerID string, targetPID uint32) (armotypes.Process, error) { pt.mutex.RLock() defer pt.mutex.RUnlock() diff --git a/pkg/processtree/creator/processtree_creator_interface.go b/pkg/processtree/creator/processtree_creator_interface.go index 1a00c34fd3..f4bb7bdaa3 100644 --- a/pkg/processtree/creator/processtree_creator_interface.go +++ b/pkg/processtree/creator/processtree_creator_interface.go @@ -19,6 +19,8 @@ type ProcessTreeCreator interface { // since boot (CLOCK_BOOTTIME), sourced exclusively from /proc//stat // field 22. Returns 0 when unknown (process not yet seen, or already exited). GetProcessBootTimeNs(pid uint32) uint64 + // GetPidBranch returns the container branch for a pid under the creator's lock + GetPidBranch(containerTree any, containerID string, targetPID uint32) (armotypes.Process, error) // Start the process tree creator and begin background tasks Start() // Stop the process tree creator and cleanup resources diff --git a/pkg/processtree/process_tree_manager.go b/pkg/processtree/process_tree_manager.go index 0d5373d1bf..dd185a63c5 100644 --- a/pkg/processtree/process_tree_manager.go +++ b/pkg/processtree/process_tree_manager.go @@ -87,7 +87,7 @@ func (ptm *ProcessTreeManagerImpl) GetContainerProcessTree(containerID string, p func() { ptm.mutex.RLock() defer ptm.mutex.RUnlock() - containerSubtree, subtreeErr = ptm.containerTree.GetPidBranch(containerID, pid, ptm.creator.GetProcessMap()) + containerSubtree, subtreeErr = ptm.creator.GetPidBranch(ptm.containerTree, containerID, pid) }() if subtreeErr != nil { diff --git a/pkg/processtree/process_tree_manager_test.go b/pkg/processtree/process_tree_manager_test.go index df04fe71a4..29f0298a21 100644 --- a/pkg/processtree/process_tree_manager_test.go +++ b/pkg/processtree/process_tree_manager_test.go @@ -3,6 +3,7 @@ package processtree import ( "fmt" "os" + "sync" "testing" "time" @@ -144,3 +145,46 @@ func TestManager_GetContainerProcessTree_CarriesStartTime(t *testing.T) { require.NoError(t, err) require.Equal(t, "nginx", bypassed.Comm) } + +func TestManager_GetContainerProcessTree_ConcurrentExitCleanup(t *testing.T) { + cfg := config.Config{ExitCleanup: treeconfig.ExitCleanupConfig{ + MaxPendingExits: 100, CleanupInterval: time.Millisecond, CleanupDelay: 0, + }} + containerTree := containerprocesstree.NewContainerProcessTree() + creator := processtreecreator.NewProcessTreeCreator(containerTree, cfg) + mgr := NewProcessTreeManager(creator, containerTree, cfg) + containerTree.ContainerCallback(containercollection.PubSubEvent{ + Type: containercollection.EventTypeAddContainer, + Container: &containercollection.Container{ + Runtime: containercollection.RuntimeMetadata{ + BasicRuntimeMetadata: eventtypes.BasicRuntimeMetadata{ContainerID: "host", ContainerPID: 1}, + }, + }, + }) + require.NoError(t, mgr.ReportEvent(utils.ProcfsEventType, &events.ProcfsEvent{PID: 1, Comm: "systemd", ContainerID: "host"})) + mgr.Start() + defer mgr.Stop() + + const parent, child uint32 = 1 << 30, 1<<30 + 1 + stop := make(chan struct{}) + var wg sync.WaitGroup + wg.Go(func() { + for { + select { + case <-stop: + return + default: + _, _ = mgr.GetContainerProcessTree("host", child, false) + } + } + }) + for range 200 { + now := time.Now().UnixNano() + require.NoError(t, mgr.ReportEvent(utils.ExecveEventType, &utils.StructEvent{ContainerID: "host", Pid: parent, Ppid: 1, Comm: "sh", Timestamp: now})) + require.NoError(t, mgr.ReportEvent(utils.ExecveEventType, &utils.StructEvent{ContainerID: "host", Pid: child, Ppid: parent, Comm: "sleep", Timestamp: now})) + require.NoError(t, mgr.ReportEvent(utils.ExitEventType, &utils.StructEvent{ContainerID: "host", Pid: parent, Ppid: 1, Timestamp: now})) + time.Sleep(2 * time.Millisecond) + } + close(stop) + wg.Wait() +}