Skip to content

Commit 2b12f00

Browse files
DeusDataLjove02
andcommitted
fix(ui): bound octree recursion for coincident points (3D layout)
Coincident (or sub-ULP-separated) bodies made octree_insert subdivide forever in the graph-UI 3D layout, calloc-ing one cell per level until the process crashed (stack overflow) or froze the machine allocating (the 34GB-swap reports). Guard distilled from #821: octree_insert now carries a depth and stops at depth 26 or half_size < 1e-4f, folding the body into the cell as a mass-weighted centroid aggregate (body_index = -1). octree_repulse already clamps d to 0.01 before the dx/d division, so folded coincident bodies get exactly zero force and no NaN. The default-cap raise bundled in #821 (DEFAULT_MAX_NODES and GRAPH_RENDER_NODE_LIMIT 2000 -> 60000) is a UX policy change deferred to its own discussion per review; HARD_MAX_NODES is raised to 200000 so opt-in CBM_UI_MAX_RENDER_NODES users get the new ceiling. Guard test layout_coincident_nodes_bounded drives the public layout API with same-file nodes whose distinct qualified names share one 32-bit FNV-1a hash (bit-identical coincident positions on every platform), in a fork+alarm child so the unfixed runaway cannot take down the runner. Refs #498, #726, #402 Co-authored-by: Ljove02 <135197334+Ljove02@users.noreply.github.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 9770803 commit 2b12f00

2 files changed

Lines changed: 157 additions & 5 deletions

File tree

src/ui/layout3d.c

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@
2525
/* ── Constants ────────────────────────────────────────────────── */
2626

2727
#define DEFAULT_MAX_NODES 2000
28-
#define HARD_MAX_NODES 10000
28+
#define HARD_MAX_NODES 200000
2929
#define BH_THETA 1.2f
30+
#define OCTREE_MAX_DEPTH 26 /* stop subdividing coincident points (OOM guard) */
31+
#define OCTREE_MIN_HALF 1e-4f /* minimum octree cell half-size */
3032

3133
/* Local optimization: gentle, preserves structure */
3234
#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
175177
*cy = n->oy + ((o & 2) ? q : -q);
176178
*cz = n->oz + ((o & 4) ? q : -q);
177179
}
178-
static void octree_insert(octree_node_t *n, int idx, float x, float y, float z, float mass) {
180+
static void octree_insert(octree_node_t *n, int idx, float x, float y, float z, float mass,
181+
int depth) {
179182
if (n->total_mass == 0.0f && n->body_index == -1) {
180183
n->body_index = idx;
181184
n->body_mass = mass;
@@ -185,6 +188,20 @@ static void octree_insert(octree_node_t *n, int idx, float x, float y, float z,
185188
n->total_mass = mass;
186189
return;
187190
}
191+
/* OOM guard: when bodies share (or nearly share) a position, subdivision
192+
* never separates them, so half_size shrinks toward zero and we allocate
193+
* octree cells without bound — the runaway that exhausted memory on large
194+
* graphs. Once we hit the depth/size floor, stop splitting and fold the body
195+
* into this cell as an aggregate (mass-weighted centroid). */
196+
if (depth >= OCTREE_MAX_DEPTH || n->half_size < OCTREE_MIN_HALF) {
197+
float nm = n->total_mass + mass;
198+
n->cx = (n->cx * n->total_mass + x * mass) / nm;
199+
n->cy = (n->cy * n->total_mass + y * mass) / nm;
200+
n->cz = (n->cz * n->total_mass + z * mass) / nm;
201+
n->total_mass = nm;
202+
n->body_index = -1;
203+
return;
204+
}
188205
if (n->body_index >= 0) {
189206
int oi = n->body_index;
190207
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,
196213
n->children[o] = octree_new(a, b, c, n->half_size * 0.5f);
197214
}
198215
if (n->children[o])
199-
octree_insert(n->children[o], oi, ox, oy, oz, om);
216+
octree_insert(n->children[o], oi, ox, oy, oz, om, depth + 1);
200217
}
201218
float nm = n->total_mass + mass;
202219
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,
210227
n->children[o] = octree_new(a, b, c, n->half_size * 0.5f);
211228
}
212229
if (n->children[o])
213-
octree_insert(n->children[o], idx, x, y, z, mass);
230+
octree_insert(n->children[o], idx, x, y, z, mass, depth + 1);
214231
}
215232
static void octree_repulse(octree_node_t *n, float px, float py, float pz, float mm, int si,
216233
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
274291
if (!root)
275292
break;
276293
for (int i = 0; i < n; i++)
277-
octree_insert(root, i, b[i].x, b[i].y, b[i].z, b[i].mass);
294+
octree_insert(root, i, b[i].x, b[i].y, b[i].z, b[i].mass, 0);
278295
for (int i = 0; i < n; i++)
279296
octree_repulse(root, b[i].x, b[i].y, b[i].z, b[i].mass, i, LOCAL_REPULSION, &b[i].fx,
280297
&b[i].fy, &b[i].fz);

tests/test_ui.c

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <string.h>
1818
#ifndef _WIN32
1919
#include <unistd.h>
20+
#include <sys/wait.h>
2021
#endif
2122

2223
/* ── Config tests ─────────────────────────────────────────────── */
@@ -454,6 +455,139 @@ TEST(layout_null_inputs) {
454455
PASS();
455456
}
456457

458+
/* ── Octree recursion guard (distilled from PR #821; refs #498/#726/#402) ── */
459+
460+
/* Bodies that share a position made octree_insert subdivide forever — the
461+
* cell around them shrinks but never separates them, so one octree cell is
462+
* calloc'd per level until the process dies (stack overflow) or freezes the
463+
* machine allocating (the 34GB-swap reports). Fixed by the depth/half-size
464+
* floor in src/ui/layout3d.c (OCTREE_MAX_DEPTH / OCTREE_MIN_HALF).
465+
*
466+
* Coincident positions are reachable through the public layout API: layout3d
467+
* anchors each node by fnv1a(file cluster key) and jitters it with a PRNG
468+
* seeded by fnv1a(qualified_name). The three QNs below are distinct strings
469+
* with IDENTICAL 32-bit FNV-1a hashes (0x06bb012e, found by offline brute
470+
* force), so in the same file they get bit-identical positions on every
471+
* platform (integer hashing only — no libm in the coincidence path).
472+
*
473+
* A literal sub-ULP-separated pair cannot be constructed through the public
474+
* API: same-anchor positions are quantized to exact multiples of the jitter
475+
* quantum (5/4096 — exactly 20 ULP at anchor magnitude ~600), and
476+
* cross-anchor separations depend on the platform's cosf/sinf bits. Exact
477+
* coincidence is the API-reachable degenerate input, and it necessarily
478+
* drives the recursion through the sub-ULP regime: half_size falls below
479+
* ULP(center) with the bodies still unseparated, freezing child centers
480+
* while cells keep being allocated.
481+
*/
482+
#if !defined(_WIN32)
483+
/* Child body: builds the store and runs the layout so a crash or hang cannot
484+
* take down the runner (alarm bounds a hang, fork isolates a SIGSEGV).
485+
* Deliberately NO memory rlimit: under a rlimit a failing calloc makes
486+
* octree_insert silently truncate and the UNFIXED code would complete —
487+
* turning this guard vacuously green. The alarm alone bounds the runaway.
488+
* Exit codes: 0 ok, 2 store setup, 3 layout NULL, 4 node count/lookup,
489+
* 5 fixture no longer coincident, 6 non-finite coordinate. Never returns. */
490+
static void layout_octree_guard_child(void) {
491+
alarm(5); /* post-fix the whole child runs in milliseconds */
492+
cbm_store_t *store = cbm_store_open_memory();
493+
if (!store)
494+
_exit(2);
495+
if (cbm_store_upsert_project(store, "test", "/tmp/test") != CBM_STORE_OK)
496+
_exit(2);
497+
498+
/* Distinct QNs, one fnv1a hash — coincident after anchor + jitter. */
499+
static const char *cqn[3] = {"test::octree_c5988474", "test::octree_c11394919",
500+
"test::octree_c33141700"};
501+
for (int i = 0; i < 3; i++) {
502+
char name[32];
503+
snprintf(name, sizeof(name), "co%d", i);
504+
cbm_node_t n = {.project = "test",
505+
.label = "Function",
506+
.name = name,
507+
.qualified_name = cqn[i],
508+
.file_path = "pkg/sub/mod/a.c",
509+
.start_line = i + 1,
510+
.end_line = i + 2};
511+
if (cbm_store_upsert_node(store, &n) <= 0)
512+
_exit(2);
513+
}
514+
/* A few normally-spread nodes so the octree root box has realistic
515+
* (non-degenerate) extent, as in the reported repositories. */
516+
for (int i = 0; i < 3; i++) {
517+
char name[32], qn[64], fp[32];
518+
snprintf(name, sizeof(name), "fn%d", i);
519+
snprintf(qn, sizeof(qn), "test::spread_fn%d", i);
520+
snprintf(fp, sizeof(fp), "dir%d/f%d.c", i, i);
521+
cbm_node_t n = {.project = "test",
522+
.label = "Function",
523+
.name = name,
524+
.qualified_name = qn,
525+
.file_path = fp,
526+
.start_line = 1,
527+
.end_line = 2};
528+
if (cbm_store_upsert_node(store, &n) <= 0)
529+
_exit(2);
530+
}
531+
532+
cbm_layout_result_t *r = cbm_layout_compute(store, "test", CBM_LAYOUT_OVERVIEW, NULL, 0, 100);
533+
if (!r)
534+
_exit(3);
535+
if (r->node_count != 6)
536+
_exit(4);
537+
538+
/* The colliding QNs must actually be coincident — identical output
539+
* coordinates (identical seeds → identical positions, and coincident
540+
* bodies receive identical forces every iteration, so they stay
541+
* together). If a seeding change ever breaks this, the fixture no longer
542+
* reproduces the bug: fail loudly instead of going vacuously green. */
543+
int ci[3], nc = 0;
544+
for (int i = 0; i < r->node_count && nc < 3; i++) {
545+
if (r->nodes[i].qualified_name &&
546+
strncmp(r->nodes[i].qualified_name, "test::octree_c", 14) == 0)
547+
ci[nc++] = i;
548+
}
549+
if (nc != 3)
550+
_exit(4);
551+
for (int k = 1; k < 3; k++) {
552+
if (r->nodes[ci[k]].x != r->nodes[ci[0]].x || r->nodes[ci[k]].y != r->nodes[ci[0]].y ||
553+
r->nodes[ci[k]].z != r->nodes[ci[0]].z)
554+
_exit(5);
555+
}
556+
for (int i = 0; i < r->node_count; i++) {
557+
if (!isfinite(r->nodes[i].x) || !isfinite(r->nodes[i].y) || !isfinite(r->nodes[i].z))
558+
_exit(6);
559+
}
560+
561+
cbm_layout_free(r);
562+
cbm_store_close(store);
563+
_exit(0);
564+
}
565+
#endif
566+
567+
TEST(layout_coincident_nodes_bounded) {
568+
#if defined(_WIN32)
569+
SKIP_PLATFORM("fork/alarm not available; POSIX-only bounded-hang reproduction");
570+
#else
571+
fflush(NULL);
572+
pid_t pid = fork();
573+
if (pid < 0)
574+
FAIL("fork() failed");
575+
if (pid == 0)
576+
layout_octree_guard_child(); /* never returns */
577+
578+
int status = 0;
579+
(void)waitpid(pid, &status, 0);
580+
581+
/* Unfixed code dies here: SIGSEGV (unbounded recursion overflowing the
582+
* stack) or SIGALRM (tail-call-optimized allocation runaway cut off by
583+
* the child's alarm). Fixed code exits 0 well within the budget. */
584+
ASSERT_FALSE(WIFSIGNALED(status));
585+
ASSERT_TRUE(WIFEXITED(status));
586+
ASSERT_EQ(WEXITSTATUS(status), 0);
587+
PASS();
588+
#endif
589+
}
590+
457591
/* ── Suite ────────────────────────────────────────────────────── */
458592

459593
SUITE(ui) {
@@ -477,4 +611,5 @@ SUITE(ui) {
477611
RUN_TEST(layout_deterministic);
478612
RUN_TEST(layout_to_json);
479613
RUN_TEST(layout_null_inputs);
614+
RUN_TEST(layout_coincident_nodes_bounded);
480615
}

0 commit comments

Comments
 (0)