Merge pull request DeusData#848 from DeusData/distill/821-octree-guard

fix(ui): bound octree recursion for coincident points (3D layout)

Author: DeusData

src/ui/layout3d.c

@@ -25,8 +25,10 @@
 /* ── Constants ────────────────────────────────────────────────── */
 
 #define DEFAULT_MAX_NODES 2000
-#define HARD_MAX_NODES 10000
+#define HARD_MAX_NODES 200000
 #define BH_THETA 1.2f
+#define OCTREE_MAX_DEPTH 26   /* stop subdividing coincident points (OOM guard) */
+#define OCTREE_MIN_HALF 1e-4f /* minimum octree cell half-size */
 
 /* Local optimization: gentle, preserves structure */
 #define LOCAL_REPULSION 8.0f
@@ -175,7 +177,8 @@ static void child_center(octree_node_t *n, int o, float *cx, float *cy, float *c
     *cy = n->oy + ((o & 2) ? q : -q);
     *cz = n->oz + ((o & 4) ? q : -q);
 }
-static void octree_insert(octree_node_t *n, int idx, float x, float y, float z, float mass) {
+static void octree_insert(octree_node_t *n, int idx, float x, float y, float z, float mass,
+                          int depth) {
     if (n->total_mass == 0.0f && n->body_index == -1) {
         n->body_index = idx;
         n->body_mass = mass;
@@ -185,6 +188,20 @@ static void octree_insert(octree_node_t *n, int idx, float x, float y, float z,
         n->total_mass = mass;
         return;
     }
+    /* OOM guard: when bodies share (or nearly share) a position, subdivision
+     * never separates them, so half_size shrinks toward zero and we allocate
+     * octree cells without bound — the runaway that exhausted memory on large
+     * graphs. Once we hit the depth/size floor, stop splitting and fold the body
+     * into this cell as an aggregate (mass-weighted centroid). */
+    if (depth >= OCTREE_MAX_DEPTH || n->half_size < OCTREE_MIN_HALF) {
+        float nm = n->total_mass + mass;
+        n->cx = (n->cx * n->total_mass + x * mass) / nm;
+        n->cy = (n->cy * n->total_mass + y * mass) / nm;
+        n->cz = (n->cz * n->total_mass + z * mass) / nm;
+        n->total_mass = nm;
+        n->body_index = -1;
+        return;
+    }
     if (n->body_index >= 0) {
         int oi = n->body_index;
         float ox = n->cx, oy = n->cy, oz = n->cz, om = n->body_mass;
@@ -196,7 +213,7 @@ static void octree_insert(octree_node_t *n, int idx, float x, float y, float z,
             n->children[o] = octree_new(a, b, c, n->half_size * 0.5f);
         }
         if (n->children[o])
-            octree_insert(n->children[o], oi, ox, oy, oz, om);
+            octree_insert(n->children[o], oi, ox, oy, oz, om, depth + 1);
     }
     float nm = n->total_mass + mass;
     n->cx = (n->cx * n->total_mass + x * mass) / nm;
@@ -210,7 +227,7 @@ static void octree_insert(octree_node_t *n, int idx, float x, float y, float z,
         n->children[o] = octree_new(a, b, c, n->half_size * 0.5f);
     }
     if (n->children[o])
-        octree_insert(n->children[o], idx, x, y, z, mass);
+        octree_insert(n->children[o], idx, x, y, z, mass, depth + 1);
 }
 static void octree_repulse(octree_node_t *n, float px, float py, float pz, float mm, int si,
                            float kr, float *fx, float *fy, float *fz) {
@@ -274,7 +291,7 @@ static void local_optimize(body_t *b, int n, const int *es, const int *ed, int n
         if (!root)
             break;
         for (int i = 0; i < n; i++)
-            octree_insert(root, i, b[i].x, b[i].y, b[i].z, b[i].mass);
+            octree_insert(root, i, b[i].x, b[i].y, b[i].z, b[i].mass, 0);
         for (int i = 0; i < n; i++)
             octree_repulse(root, b[i].x, b[i].y, b[i].z, b[i].mass, i, LOCAL_REPULSION, &b[i].fx,
                            &b[i].fy, &b[i].fz);

tests/test_ui.c

@@ -17,6 +17,7 @@
 #include <string.h>
 #ifndef _WIN32
 #include <unistd.h>
+#include <sys/wait.h>
 #endif
 
 /* ── Config tests ─────────────────────────────────────────────── */
@@ -454,6 +455,139 @@ TEST(layout_null_inputs) {
     PASS();
 }
 
+/* ── Octree recursion guard (distilled from PR #821; refs #498/#726/#402) ── */
+
+/* Bodies that share a position made octree_insert subdivide forever — the
+ * cell around them shrinks but never separates them, so one octree cell is
+ * calloc'd per level until the process dies (stack overflow) or freezes the
+ * machine allocating (the 34GB-swap reports). Fixed by the depth/half-size
+ * floor in src/ui/layout3d.c (OCTREE_MAX_DEPTH / OCTREE_MIN_HALF).
+ *
+ * Coincident positions are reachable through the public layout API: layout3d
+ * anchors each node by fnv1a(file cluster key) and jitters it with a PRNG
+ * seeded by fnv1a(qualified_name). The three QNs below are distinct strings
+ * with IDENTICAL 32-bit FNV-1a hashes (0x06bb012e, found by offline brute
+ * force), so in the same file they get bit-identical positions on every
+ * platform (integer hashing only — no libm in the coincidence path).
+ *
+ * A literal sub-ULP-separated pair cannot be constructed through the public
+ * API: same-anchor positions are quantized to exact multiples of the jitter
+ * quantum (5/4096 — exactly 20 ULP at anchor magnitude ~600), and
+ * cross-anchor separations depend on the platform's cosf/sinf bits. Exact
+ * coincidence is the API-reachable degenerate input, and it necessarily
+ * drives the recursion through the sub-ULP regime: half_size falls below
+ * ULP(center) with the bodies still unseparated, freezing child centers
+ * while cells keep being allocated.
+ */
+#if !defined(_WIN32)
+/* Child body: builds the store and runs the layout so a crash or hang cannot
+ * take down the runner (alarm bounds a hang, fork isolates a SIGSEGV).
+ * Deliberately NO memory rlimit: under a rlimit a failing calloc makes
+ * octree_insert silently truncate and the UNFIXED code would complete —
+ * turning this guard vacuously green. The alarm alone bounds the runaway.
+ * Exit codes: 0 ok, 2 store setup, 3 layout NULL, 4 node count/lookup,
+ * 5 fixture no longer coincident, 6 non-finite coordinate. Never returns. */
+static void layout_octree_guard_child(void) {
+    alarm(5); /* post-fix the whole child runs in milliseconds */
+    cbm_store_t *store = cbm_store_open_memory();
+    if (!store)
+        _exit(2);
+    if (cbm_store_upsert_project(store, "test", "/tmp/test") != CBM_STORE_OK)
+        _exit(2);
+
+    /* Distinct QNs, one fnv1a hash — coincident after anchor + jitter. */
+    static const char *cqn[3] = {"test::octree_c5988474", "test::octree_c11394919",
+                                 "test::octree_c33141700"};
+    for (int i = 0; i < 3; i++) {
+        char name[32];
+        snprintf(name, sizeof(name), "co%d", i);
+        cbm_node_t n = {.project = "test",
+                        .label = "Function",
+                        .name = name,
+                        .qualified_name = cqn[i],
+                        .file_path = "pkg/sub/mod/a.c",
+                        .start_line = i + 1,
+                        .end_line = i + 2};
+        if (cbm_store_upsert_node(store, &n) <= 0)
+            _exit(2);
+    }
+    /* A few normally-spread nodes so the octree root box has realistic
+     * (non-degenerate) extent, as in the reported repositories. */
+    for (int i = 0; i < 3; i++) {
+        char name[32], qn[64], fp[32];
+        snprintf(name, sizeof(name), "fn%d", i);
+        snprintf(qn, sizeof(qn), "test::spread_fn%d", i);
+        snprintf(fp, sizeof(fp), "dir%d/f%d.c", i, i);
+        cbm_node_t n = {.project = "test",
+                        .label = "Function",
+                        .name = name,
+                        .qualified_name = qn,
+                        .file_path = fp,
+                        .start_line = 1,
+                        .end_line = 2};
+        if (cbm_store_upsert_node(store, &n) <= 0)
+            _exit(2);
+    }
+
+    cbm_layout_result_t *r = cbm_layout_compute(store, "test", CBM_LAYOUT_OVERVIEW, NULL, 0, 100);
+    if (!r)
+        _exit(3);
+    if (r->node_count != 6)
+        _exit(4);
+
+    /* The colliding QNs must actually be coincident — identical output
+     * coordinates (identical seeds → identical positions, and coincident
+     * bodies receive identical forces every iteration, so they stay
+     * together). If a seeding change ever breaks this, the fixture no longer
+     * reproduces the bug: fail loudly instead of going vacuously green. */
+    int ci[3], nc = 0;
+    for (int i = 0; i < r->node_count && nc < 3; i++) {
+        if (r->nodes[i].qualified_name &&
+            strncmp(r->nodes[i].qualified_name, "test::octree_c", 14) == 0)
+            ci[nc++] = i;
+    }
+    if (nc != 3)
+        _exit(4);
+    for (int k = 1; k < 3; k++) {
+        if (r->nodes[ci[k]].x != r->nodes[ci[0]].x || r->nodes[ci[k]].y != r->nodes[ci[0]].y ||
+            r->nodes[ci[k]].z != r->nodes[ci[0]].z)
+            _exit(5);
+    }
+    for (int i = 0; i < r->node_count; i++) {
+        if (!isfinite(r->nodes[i].x) || !isfinite(r->nodes[i].y) || !isfinite(r->nodes[i].z))
+            _exit(6);
+    }
+
+    cbm_layout_free(r);
+    cbm_store_close(store);
+    _exit(0);
+}
+#endif
+
+TEST(layout_coincident_nodes_bounded) {
+#if defined(_WIN32)
+    SKIP_PLATFORM("fork/alarm not available; POSIX-only bounded-hang reproduction");
+#else
+    fflush(NULL);
+    pid_t pid = fork();
+    if (pid < 0)
+        FAIL("fork() failed");
+    if (pid == 0)
+        layout_octree_guard_child(); /* never returns */
+
+    int status = 0;
+    (void)waitpid(pid, &status, 0);
+
+    /* Unfixed code dies here: SIGSEGV (unbounded recursion overflowing the
+     * stack) or SIGALRM (tail-call-optimized allocation runaway cut off by
+     * the child's alarm). Fixed code exits 0 well within the budget. */
+    ASSERT_FALSE(WIFSIGNALED(status));
+    ASSERT_TRUE(WIFEXITED(status));
+    ASSERT_EQ(WEXITSTATUS(status), 0);
+    PASS();
+#endif
+}
+
 /* ── Suite ────────────────────────────────────────────────────── */
 
 SUITE(ui) {
@@ -477,4 +611,5 @@ SUITE(ui) {
     RUN_TEST(layout_deterministic);
     RUN_TEST(layout_to_json);
     RUN_TEST(layout_null_inputs);
+    RUN_TEST(layout_coincident_nodes_bounded);
 }