fix(subprocess): count worker descendants in their own process group in tree RSS

Addresses CodeRabbit review on PR #34 (src/foundation/subprocess.c).

The index worker's RSS probe summed only processes whose process group
matched the worker's. Every child spawned through cbm_subprocess gets its
own group (setpgid on Linux, POSIX_SPAWN_SETPGROUP on macOS), including
`git ls-files -z` from discover.c. Discovery memory on a large Git index
was therefore invisible to the index_max_rss_mb watchdog.

Linux: the /proc scan now also reads ppid, collects processes into a
bounded array (262144 entries, fails closed past it), and marks the
worker group plus every descendant by parent pid (sorted + bsearch,
repeated until no change). Each process is summed once; the existing
unreadable-root, gone and empty outcomes are unchanged.

macOS: the PROC_PGRP_ONLY listing is expanded with PROC_PPID_ONLY children
of each collected pid, deduplicated, capped at 65536 and failing closed.
Not compiled or run in this change; Linux only was verified.

Windows is unchanged: the Job Object already covers descendants.

Test: subprocess_tree_rss_counts_descendants_in_their_own_process_group
spawns a real tree whose grandchild moves to its own group and holds
128 MiB, and asserts the measured total grows by at least 96 MiB. The
__cbm_rss_tree_root helper role gains an optional `detach` argument; the
detached holder exits when its parent disappears (alarm(120) backstop).

Author: ngallodev

src/foundation/subprocess.c

@@ -591,7 +591,7 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces
  * all-process scan cannot decide membership: proc_pidinfo answers EPERM for
  * every process of another user, so "not in our group" and "in our group but
  * unreadable" would look the same. Returns the member count, or -1. */
-static int cbm_tree_rss_list_group(pid_t pgid, pid_t **pids_out) {
+static int cbm_tree_rss_list_pids(uint32_t type, uint32_t arg, pid_t **pids_out) {
     int capacity = 64;
     pid_t *pids = NULL;
     for (;;) {
@@ -602,8 +602,7 @@ static int cbm_tree_rss_list_group(pid_t pgid, pid_t **pids_out) {
         }
         pids = grown;
         errno = 0;
-        int bytes =
-            proc_listpids(PROC_PGRP_ONLY, (uint32_t)pgid, pids, capacity * (int)sizeof(*pids));
+        int bytes = proc_listpids(type, arg, pids, capacity * (int)sizeof(*pids));
         if (bytes < 0 || (bytes == 0 && errno != 0)) {
             cbm_free(CBM_MEM_CLASS_OTHER, pids);
             return -1;
@@ -621,6 +620,63 @@ static int cbm_tree_rss_list_group(pid_t pgid, pid_t **pids_out) {
     }
 }
 
+/* Bound on the processes one probe retains; past it the probe fails closed. */
+enum { CBM_APPLE_PROBE_MAX_PROCS = 65536 };
+
+/* The worker's process group plus every descendant by parent pid. Children
+ * placed in a group of their own (git discovery) are not in the group list.
+ * Pids already collected are skipped, so nothing is counted twice. Returns the
+ * count, or -1. */
+static int cbm_tree_rss_list_tree(pid_t pgid, pid_t **pids_out) {
+    pid_t *tree = NULL;
+    int count = cbm_tree_rss_list_pids(PROC_PGRP_ONLY, (uint32_t)pgid, &tree);
+    if (count < 0) {
+        return -1;
+    }
+    int capacity = count > 0 ? count : 1;
+    for (int index = 0; index < count; index++) {
+        pid_t *children = NULL;
+        int child_count = tree[index] > 0 ? cbm_tree_rss_list_pids(PROC_PPID_ONLY,
+                                                                   (uint32_t)tree[index], &children)
+                                          : 0;
+        if (child_count < 0) {
+            /* ESRCH-style races surface as an empty listing; a real error
+             * must not under-report the tree. */
+            cbm_free(CBM_MEM_CLASS_OTHER, tree);
+            return -1;
+        }
+        for (int child = 0; child < child_count; child++) {
+            bool known = children[child] <= 0;
+            for (int seen = 0; !known && seen < count; seen++) {
+                known = tree[seen] == children[child];
+            }
+            if (known) {
+                continue;
+            }
+            if (count >= CBM_APPLE_PROBE_MAX_PROCS) {
+                cbm_free(CBM_MEM_CLASS_OTHER, children);
+                cbm_free(CBM_MEM_CLASS_OTHER, tree);
+                return -1;
+            }
+            if (count == capacity) {
+                pid_t *grown =
+                    cbm_realloc(CBM_MEM_CLASS_OTHER, tree, (size_t)capacity * 2 * sizeof(*tree));
+                if (!grown) {
+                    cbm_free(CBM_MEM_CLASS_OTHER, children);
+                    cbm_free(CBM_MEM_CLASS_OTHER, tree);
+                    return -1;
+                }
+                tree = grown;
+                capacity *= 2;
+            }
+            tree[count++] = children[child];
+        }
+        cbm_free(CBM_MEM_CLASS_OTHER, children);
+    }
+    *pids_out = tree;
+    return count;
+}
+
 static int cbm_tree_rss_member_rusage(pid_t pid, bool root, struct rusage_info_v2 *usage) {
 #ifdef CBM_ENABLE_TEST_SEAMS
     int fault = cbm_tree_rss_member_fault(root);
@@ -637,7 +693,7 @@ static int cbm_tree_rss_member_rusage(pid_t pid, bool root, struct rusage_info_v
 static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process,
                                                                    uint64_t *rss_bytes) {
     pid_t *pids = NULL;
-    int count = cbm_tree_rss_list_group(process->pgid, &pids);
+    int count = cbm_tree_rss_list_tree(process->pgid, &pids);
     if (count < 0) {
         return CBM_PROC_TREE_RSS_ERROR;
     }
@@ -730,8 +786,8 @@ static cbm_linux_stat_t cbm_linux_proc_stat_line(const char *pid_name, bool root
     return CBM_LINUX_STAT_OK;
 }
 
-static cbm_linux_stat_t cbm_linux_proc_stat(const char *pid_name, bool root, pid_t *group,
-                                            int64_t *rss_pages) {
+static cbm_linux_stat_t cbm_linux_proc_stat(const char *pid_name, bool root, pid_t *parent,
+                                            pid_t *group, int64_t *rss_pages) {
     char stat_line[4096];
     cbm_linux_stat_t status =
         cbm_linux_proc_stat_line(pid_name, root, stat_line, sizeof(stat_line));
@@ -746,16 +802,20 @@ static cbm_linux_stat_t cbm_linux_proc_stat(const char *pid_name, bool root, pid
     char *save = NULL;
     char *token = strtok_r(command_end + 2, " ", &save);
     int field = 3;
+    bool have_parent = false;
     bool have_group = false;
     bool have_rss = false;
     while (token) {
-        if (field == 5 || field == 24) {
+        if (field == 4 || field == 5 || field == 24) {
             char *end = NULL;
             long long value = strtoll(token, &end, 10);
             if (!end || *end != '\0') {
                 return CBM_LINUX_STAT_UNREADABLE;
             }
-            if (field == 5) {
+            if (field == 4) {
+                *parent = (pid_t)value;
+                have_parent = true;
+            } else if (field == 5) {
                 *group = (pid_t)value;
                 have_group = true;
             } else {
@@ -767,7 +827,76 @@ static cbm_linux_stat_t cbm_linux_proc_stat(const char *pid_name, bool root, pid
         token = strtok_r(NULL, " ", &save);
         field++;
     }
-    return have_group && have_rss ? CBM_LINUX_STAT_OK : CBM_LINUX_STAT_UNREADABLE;
+    return have_parent && have_group && have_rss ? CBM_LINUX_STAT_OK : CBM_LINUX_STAT_UNREADABLE;
+}
+
+/* One scanned process. `counted` is set once it is known to belong to the
+ * worker tree. */
+typedef struct {
+    pid_t pid;
+    pid_t parent;
+    pid_t group;
+    uint64_t bytes;
+    bool counted;
+} cbm_linux_proc_t;
+
+/* Bound on the processes one probe retains. Past it the probe fails closed
+ * instead of reporting a total that may miss part of the tree. */
+enum { CBM_LINUX_PROBE_MAX_PROCS = 262144, CBM_LINUX_PROBE_INITIAL_PROCS = 512 };
+
+static bool cbm_linux_probe_push(cbm_linux_proc_t **procs, size_t *count, size_t *capacity,
+                                 const cbm_linux_proc_t *proc) {
+    if (*count == *capacity) {
+        if (*capacity >= CBM_LINUX_PROBE_MAX_PROCS) {
+            return false;
+        }
+        size_t grown_capacity = *capacity ? *capacity * 2 : CBM_LINUX_PROBE_INITIAL_PROCS;
+        cbm_linux_proc_t *grown =
+            cbm_realloc(CBM_MEM_CLASS_OTHER, *procs, grown_capacity * sizeof(**procs));
+        if (!grown) {
+            return false;
+        }
+        *procs = grown;
+        *capacity = grown_capacity;
+    }
+    (*procs)[(*count)++] = *proc;
+    return true;
+}
+
+static int cbm_linux_probe_by_pid(const void *left, const void *right) {
+    pid_t a = ((const cbm_linux_proc_t *)left)->pid;
+    pid_t b = ((const cbm_linux_proc_t *)right)->pid;
+    return (a > b) - (a < b);
+}
+
+/* Mark every process of the worker tree: members of its process group, plus
+ * any descendant by parent pid. Descendants can sit in a group of their own
+ * (git discovery children are spawned with setpgid), so the group alone would
+ * miss them. `procs` is sorted by pid, so a parent is found by binary search
+ * and each pass is O(n log n); passes repeat until no process is added (at
+ * most the tree depth). A process is counted once because the flag is checked
+ * before it is set. */
+static void cbm_linux_probe_mark_tree(cbm_linux_proc_t *procs, size_t count, pid_t pgid) {
+    qsort(procs, count, sizeof(*procs), cbm_linux_probe_by_pid);
+    for (size_t index = 0; index < count; index++) {
+        procs[index].counted = procs[index].group == pgid;
+    }
+    bool changed = true;
+    while (changed) {
+        changed = false;
+        for (size_t index = 0; index < count; index++) {
+            if (procs[index].counted || procs[index].parent <= 1) {
+                continue;
+            }
+            cbm_linux_proc_t key = {.pid = procs[index].parent};
+            const cbm_linux_proc_t *parent =
+                bsearch(&key, procs, count, sizeof(*procs), cbm_linux_probe_by_pid);
+            if (parent && parent->counted) {
+                procs[index].counted = true;
+                changed = true;
+            }
+        }
+    }
 }
 
 static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process,
@@ -782,36 +911,54 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces
         return CBM_PROC_TREE_RSS_ERROR;
     }
 
-    uint64_t total = 0;
-    int measured = 0;
+    cbm_linux_proc_t *procs = NULL;
+    size_t count = 0;
+    size_t capacity = 0;
     bool failed = false;
     struct dirent *entry;
     while (!failed && (entry = readdir(proc)) != NULL) {
         if (!cbm_proc_pid_name(entry->d_name)) {
             continue;
         }
+        pid_t parent = 0;
         pid_t group = 0;
         int64_t pages = 0;
-        bool root_entry = strtol(entry->d_name, NULL, 10) == (long)process->pid;
-        cbm_linux_stat_t status = cbm_linux_proc_stat(entry->d_name, root_entry, &group, &pages);
+        long pid = strtol(entry->d_name, NULL, 10);
+        bool root_entry = pid == (long)process->pid;
+        cbm_linux_stat_t status =
+            cbm_linux_proc_stat(entry->d_name, root_entry, &parent, &group, &pages);
         if (status != CBM_LINUX_STAT_OK) {
             failed = root_entry || status != CBM_LINUX_STAT_GONE;
             continue;
         }
-        if (group != process->pgid || pages < 0) {
+        if (pages < 0) {
             continue;
         }
         uint64_t page_count = (uint64_t)pages;
-        uint64_t bytes = page_count > UINT64_MAX / (uint64_t)page_size
-                             ? UINT64_MAX
-                             : page_count * (uint64_t)page_size;
-        cbm_rss_add_saturated(&total, bytes);
-        measured++;
+        cbm_linux_proc_t item = {.pid = (pid_t)pid,
+                                 .parent = parent,
+                                 .group = group,
+                                 .bytes = page_count > UINT64_MAX / (uint64_t)page_size
+                                              ? UINT64_MAX
+                                              : page_count * (uint64_t)page_size};
+        failed = !cbm_linux_probe_push(&procs, &count, &capacity, &item);
     }
     (void)closedir(proc);
     if (failed) {
+        cbm_free(CBM_MEM_CLASS_OTHER, procs);
         return CBM_PROC_TREE_RSS_ERROR;
     }
+
+    cbm_linux_probe_mark_tree(procs, count, process->pgid);
+    uint64_t total = 0;
+    int measured = 0;
+    for (size_t index = 0; index < count; index++) {
+        if (procs[index].counted) {
+            cbm_rss_add_saturated(&total, procs[index].bytes);
+            measured++;
+        }
+    }
+    cbm_free(CBM_MEM_CLASS_OTHER, procs);
     if (measured == 0) {
         return CBM_PROC_TREE_RSS_EMPTY;
     }

tests/test_main.c

@@ -2044,7 +2044,7 @@ static int tf_role_env_isolation(int argc, char **argv) {
  * publishes its pid by an atomic rename once every page is touched, then
  * blocks until it is killed. The root exits when its descendant dies, so a
  * failed setup ends the tree instead of leaving the test waiting. */
-static int tf_rss_tree_hold(const char *ready_path, const char *mib_text) {
+static int tf_rss_tree_hold(const char *ready_path, const char *mib_text, bool own_group) {
     char *end = NULL;
     long mib = strtol(mib_text, &end, 10);
     if (!end || *end != '\0' || mib < 0 || mib > 1024) {
@@ -2074,6 +2074,18 @@ static int tf_rss_tree_hold(const char *ready_path, const char *mib_text) {
     }
     volatile unsigned char keep = block ? block[bytes - 1] : 0;
     (void)keep;
+#ifndef _WIN32
+    if (own_group) {
+        /* Nothing kills a process outside the worker's group, so leave when
+         * the root (the parent) is gone, with an alarm as a backstop. */
+        pid_t parent = getppid();
+        (void)alarm(120);
+        while (getppid() == parent) {
+            cbm_usleep(10000);
+        }
+        return 0;
+    }
+#endif
     for (;;) {
 #ifdef _WIN32
         Sleep(INFINITE);
@@ -2084,11 +2096,13 @@ static int tf_rss_tree_hold(const char *ready_path, const char *mib_text) {
 }
 
 static int tf_role_rss_tree_hold(int argc, char **argv) {
-    return argc == 4 ? tf_rss_tree_hold(argv[2], argv[3]) : -1;
+    return argc == 4 ? tf_rss_tree_hold(argv[2], argv[3], false) : -1;
 }
 
 static int tf_role_rss_tree_root(int argc, char **argv) {
-    if (argc != 4) {
+    /* A fifth argument "detach" puts the descendant in a process group of its
+     * own, as git discovery children are. */
+    if (argc != 4 && argc != 5) {
         return -1;
     }
 #ifdef _WIN32
@@ -2123,7 +2137,10 @@ static int tf_role_rss_tree_root(int argc, char **argv) {
         return 85;
     }
     if (child == 0) {
-        _exit(tf_rss_tree_hold(argv[2], argv[3]));
+        if (argc == 5 && setpgid(0, 0) != 0) {
+            _exit(86);
+        }
+        _exit(tf_rss_tree_hold(argv[2], argv[3], argc == 5));
     }
     int status = 0;
     while (waitpid(child, &status, 0) < 0 && errno == EINTR) {}

tests/test_subprocess.c

@@ -885,7 +885,7 @@ static bool rss_tree_finish(cbm_subprocess_t *process, rss_tree_sample_t *sample
 
 /* Measure one tree whose descendant holds `mib` MiB, with every non-root
  * member's measurement failing with `member_fault` (0 = real calls). */
-static rss_tree_sample_t rss_tree_sample(int mib, int member_fault) {
+static rss_tree_sample_t rss_tree_sample_in(int mib, int member_fault, bool own_group) {
     rss_tree_sample_t sample = {.status = CBM_PROC_TREE_RSS_ERROR,
                                 .final_status = CBM_PROC_TREE_RSS_ERROR,
                                 .final_rss = UINT64_MAX};
@@ -898,7 +898,12 @@ static rss_tree_sample_t rss_tree_sample(int mib, int member_fault) {
         return sample;
     }
     (void)snprintf(mib_text, sizeof(mib_text), "%d", mib);
-    const char *argv[] = {cbm_test_program_path, "__cbm_rss_tree_root", ready_path, mib_text, NULL};
+    const char *argv[] = {cbm_test_program_path,
+                          "__cbm_rss_tree_root",
+                          ready_path,
+                          mib_text,
+                          own_group ? "detach" : NULL,
+                          NULL};
     cbm_proc_opts_t opts = {0};
     opts.bin = cbm_test_program_path;
     opts.argv = argv;
@@ -918,6 +923,10 @@ static rss_tree_sample_t rss_tree_sample(int mib, int member_fault) {
     return sample;
 }
 
+static rss_tree_sample_t rss_tree_sample(int mib, int member_fault) {
+    return rss_tree_sample_in(mib, member_fault, false);
+}
+
 /* Only the DESCENDANT allocates, so the difference between two otherwise
  * identical trees is memory the probe can only have counted by measuring past
  * the root. 128 MiB touched against a 96 MiB floor leaves room for allocator
@@ -945,6 +954,26 @@ TEST(subprocess_tree_rss_measures_contained_descendants) {
     PASS();
 }
 
+/* git discovery children run in a process group of their own. The probe must
+ * still count them: the memory is the worker's, and a group-only probe would
+ * let it grow past the limit unseen. POSIX only (a Job Object has no groups). */
+TEST(subprocess_tree_rss_counts_descendants_in_their_own_process_group) {
+#ifdef _WIN32
+    SKIP_PLATFORM("process groups are POSIX; the Job Object probe already covers descendants");
+#else
+    rss_tree_sample_t idle = rss_tree_sample_in(0, 0, true);
+    rss_tree_sample_t loaded = rss_tree_sample_in(128, 0, true);
+    ASSERT_TRUE(idle.ready);
+    ASSERT_EQ(idle.status, CBM_PROC_TREE_RSS_OK);
+    ASSERT_TRUE(loaded.ready);
+    ASSERT_EQ(loaded.status, CBM_PROC_TREE_RSS_OK);
+    ASSERT_TRUE(loaded.rss >= idle.rss + 96 * RSS_TREE_MIB);
+    ASSERT_TRUE(idle.quiesced);
+    ASSERT_TRUE(loaded.quiesced);
+    PASS();
+#endif
+}
+
 /* A live member the platform will not let us measure must fail the probe: a
  * total without it would under-report the tree. */
 TEST(subprocess_tree_rss_fails_when_a_live_member_cannot_be_measured) {
@@ -2817,6 +2846,7 @@ SUITE(subprocess) {
     RUN_TEST(subprocess_run_null_bin_rejected);
     RUN_TEST(subprocess_spawn_returns_while_child_is_running);
     RUN_TEST(subprocess_tree_rss_measures_contained_descendants);
+    RUN_TEST(subprocess_tree_rss_counts_descendants_in_their_own_process_group);
     RUN_TEST(subprocess_tree_rss_fails_when_a_live_member_cannot_be_measured);
     RUN_TEST(subprocess_tree_rss_skips_a_member_that_exited_mid_probe);
     RUN_TEST(subprocess_natural_completion_is_cached_across_polls);