Skip to content

Commit 6bd11ff

Browse files
committed
fix(cypher): add a wall-clock execution deadline to abort runaway queries
An unbounded whole-graph OPTIONAL MATCH or a GROUP BY that produces roughly one group per node does O(bindings x groups) work in execute_return_agg and can run for minutes on a large graph before emitting a single row. The 100k row ceiling never fires (no rows are produced), so query_graph just hangs with no partial result and no error (#601). Arm a monotonic wall-clock budget (default 30s) at query entry and check it (throttled, every 1024 iterations) in the scan, relationship-expansion and aggregation hot loops. On expiry the query aborts with a clear, actionable error instead of hanging. A thread-local test hook forces the budget so the guard is covered by a deterministic reproduce-first test; a companion test confirms the default budget does not false-positive on a normal query. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent e2d41d9 commit 6bd11ff

3 files changed

Lines changed: 127 additions & 0 deletions

File tree

src/cypher/cypher.c

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2650,6 +2650,49 @@ static void rb_add_row(result_builder_t *rb, const char **values) {
26502650
* Prevents accidental multi-GB JSON payloads from unbounded MATCH (n) RETURN n. */
26512651
#define CYPHER_RESULT_CEILING 100000
26522652

2653+
/* Wall-clock execution deadline (#601). The row ceiling above only fires once
2654+
* rows exist, but an unbounded `OPTIONAL MATCH` over the full node set (or a
2655+
* GROUP BY with ~one group per node) does O(bindings x groups) work and can run
2656+
* for minutes — exhausting RAM/CPU — before a single row is produced, so the
2657+
* ceiling never trips and the caller just hangs. A monotonic deadline aborts
2658+
* such runaway queries with a clear, actionable error. Checked (throttled) in
2659+
* the scan, expansion and aggregation hot loops. */
2660+
#define CYPHER_DEADLINE_BUDGET_MS 30000 /* 30s: generous for legit heavy queries */
2661+
#define CYPHER_DEADLINE_CHECK_MASK 0x3FF /* sample the clock every 1024 iterations */
2662+
2663+
static _Thread_local uint64_t g_cypher_deadline_ms = 0; /* absolute; 0 = disarmed */
2664+
static _Thread_local bool g_cypher_timed_out = false;
2665+
static _Thread_local int64_t g_cypher_deadline_override_ms = -1; /* test hook; <0 = default */
2666+
2667+
static void cypher_deadline_arm(void) {
2668+
g_cypher_timed_out = false;
2669+
int64_t budget = g_cypher_deadline_override_ms >= 0 ? g_cypher_deadline_override_ms
2670+
: CYPHER_DEADLINE_BUDGET_MS;
2671+
g_cypher_deadline_ms = cbm_now_ms() + (uint64_t)budget;
2672+
}
2673+
2674+
/* True once the query has run past its wall-clock budget. Sticky: after the
2675+
* first trip every subsequent call returns true, so later loops short-circuit. */
2676+
static bool cypher_deadline_exceeded(void) {
2677+
if (g_cypher_timed_out) {
2678+
return true;
2679+
}
2680+
if (g_cypher_deadline_ms == 0) {
2681+
return false;
2682+
}
2683+
if (cbm_now_ms() >= g_cypher_deadline_ms) {
2684+
g_cypher_timed_out = true;
2685+
return true;
2686+
}
2687+
return false;
2688+
}
2689+
2690+
/* Test-only: force the execution budget (ms) for subsequent queries on this
2691+
* thread. 0 = trip on the first hot-loop check; <0 restores the default. */
2692+
void cbm_cypher_test_set_deadline_ms(int64_t budget_ms) {
2693+
g_cypher_deadline_override_ms = budget_ms;
2694+
}
2695+
26532696
/* ── Binding virtual variables (for WITH clause) ──────────────── */
26542697

26552698
static const char *binding_get_virtual(binding_t *b, const char *var, const char *prop) {
@@ -3054,6 +3097,11 @@ static void expand_pattern_rels(cbm_store_t *store, cbm_pattern_t *pat, binding_
30543097
int *bind_count, const int *bind_cap, const char **var_name,
30553098
bool is_optional) {
30563099
for (int ri = 0; ri < pat->rel_count; ri++) {
3100+
/* #601: stop expanding further hops once the wall-clock budget is spent
3101+
* (an unbounded expansion is exactly what blows up here). */
3102+
if (cypher_deadline_exceeded()) {
3103+
return;
3104+
}
30573105
cbm_rel_pattern_t *rel = &pat->rels[ri];
30583106
cbm_node_pattern_t *target_node = &pat->nodes[ri + SKIP_ONE];
30593107
const char *to_var = target_node->variable ? target_node->variable : "_n_t";
@@ -3068,6 +3116,9 @@ static void expand_pattern_rels(cbm_store_t *store, cbm_pattern_t *pat, binding_
30683116
int new_count = 0;
30693117

30703118
for (int bi = 0; bi < *bind_count; bi++) {
3119+
if ((bi & CYPHER_DEADLINE_CHECK_MASK) == 0 && cypher_deadline_exceeded()) {
3120+
break;
3121+
}
30713122
binding_t *b = &(*bindings)[bi];
30723123
cbm_node_t *src = binding_get(b, *var_name);
30733124
if (!src) {
@@ -4111,6 +4162,11 @@ static void execute_return_agg(cbm_return_clause_t *ret, binding_t *bindings, in
41114162
int agg_count = 0;
41124163

41134164
for (int bi = 0; bi < bind_count; bi++) {
4165+
/* #601: grouping is O(bindings x groups) — the dominant cost on a
4166+
* whole-graph GROUP BY. Abort if we blow the wall-clock budget. */
4167+
if ((bi & CYPHER_DEADLINE_CHECK_MASK) == 0 && cypher_deadline_exceeded()) {
4168+
break;
4169+
}
41144170
char key[CBM_SZ_1K] = "";
41154171
const char *vals[CBM_SZ_32];
41164172
char valbufs[CBM_SZ_32][CBM_SZ_512];
@@ -4479,6 +4535,9 @@ static int execute_single(cbm_store_t *store, cbm_query_t *q, const char *projec
44794535
const char *var_name = pat0->nodes[0].variable ? pat0->nodes[0].variable : "_n0";
44804536

44814537
for (int i = 0; i < scan_count && bind_count < bind_cap; i++) {
4538+
if ((i & CYPHER_DEADLINE_CHECK_MASK) == 0 && cypher_deadline_exceeded()) {
4539+
break;
4540+
}
44824541
binding_t b = {0};
44834542
b.store = store;
44844543
binding_set(&b, var_name, &scanned[i]);
@@ -4527,6 +4586,7 @@ int cbm_cypher_execute(cbm_store_t *store, const char *query, const char *projec
45274586
cbm_cypher_result_t *out) {
45284587
memset(out, 0, sizeof(*out));
45294588
g_cypher_depth_clamped = 0;
4589+
cypher_deadline_arm(); /* #601: start the wall-clock budget for this query */
45304590
if (max_rows <= 0) {
45314591
max_rows = CYPHER_RESULT_CEILING;
45324592
}
@@ -4570,6 +4630,18 @@ int cbm_cypher_execute(cbm_store_t *store, const char *query, const char *projec
45704630
rb_apply_distinct(&rb);
45714631
}
45724632

4633+
/* #601: abort a runaway query that blew the wall-clock budget before it can
4634+
* return a misleading partial result. Checked before the row ceiling. */
4635+
if (g_cypher_timed_out) {
4636+
rb_free(&rb);
4637+
cbm_query_free(q);
4638+
out->error =
4639+
heap_strdup("query exceeded the execution time limit — narrow the pattern with a WHERE "
4640+
"filter, use a directed MATCH instead of an unbounded OPTIONAL MATCH, or "
4641+
"add LIMIT");
4642+
return CBM_NOT_FOUND;
4643+
}
4644+
45734645
/* Check ceiling */
45744646
if (rb.row_count >= CYPHER_RESULT_CEILING) {
45754647
rb_free(&rb);

src/cypher/cypher.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,4 +332,9 @@ int cbm_cypher_parse(const char *query, cbm_query_t **out, char **error);
332332
/* Free a query AST. */
333333
void cbm_query_free(cbm_query_t *q);
334334

335+
/* Test-only (#601): force the wall-clock execution budget in milliseconds for
336+
* subsequent queries on the calling thread. 0 = trip on the first hot-loop
337+
* check; a negative value restores the default budget. */
338+
void cbm_cypher_test_set_deadline_ms(int64_t budget_ms);
339+
335340
#endif /* CBM_CYPHER_H */

tests/test_cypher.c

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2827,6 +2827,54 @@ TEST(cypher_wide_return_projection_bounded) {
28272827
#endif
28282828
}
28292829

2830+
/* #601: an unbounded whole-graph OPTIONAL MATCH / GROUP BY does
2831+
* O(bindings x groups) work and can run for minutes with no wall-clock guard —
2832+
* the 100k row ceiling never fires because no rows are produced, so query_graph
2833+
* just hangs. With the execution deadline armed to trip immediately (budget 0),
2834+
* the runaway query must abort with a clear error instead of returning a
2835+
* (misleading, possibly partial) result.
2836+
*
2837+
* RED on unfixed code: no deadline exists, so the query completes and returns
2838+
* rc==0 with rows and no error — the assertions below fail. */
2839+
TEST(cypher_exec_deadline_aborts_runaway_query_issue601) {
2840+
cbm_store_t *s = setup_cypher_store();
2841+
cbm_cypher_result_t r = {0};
2842+
2843+
cbm_cypher_test_set_deadline_ms(0); /* trip on the first hot-loop check */
2844+
int rc = cbm_cypher_execute(
2845+
s, "MATCH (a) OPTIONAL MATCH (a)-[:CALLS]->(b) RETURN a.qualified_name, count(b)", "test",
2846+
0, &r);
2847+
cbm_cypher_test_set_deadline_ms(-1); /* restore default before asserting (thread-local) */
2848+
2849+
ASSERT_TRUE(rc != 0); /* CBM_NOT_FOUND (-1) — query aborted, not success */
2850+
ASSERT_NOT_NULL(r.error);
2851+
ASSERT_TRUE(strstr(r.error, "time limit") != NULL);
2852+
ASSERT_EQ(r.row_count, 0);
2853+
2854+
cbm_cypher_result_free(&r);
2855+
cbm_store_close(s);
2856+
PASS();
2857+
}
2858+
2859+
/* #601 companion: the default (ample) budget must NOT false-positive on a
2860+
* normal small query — it still returns its rows. */
2861+
TEST(cypher_exec_deadline_allows_normal_query_issue601) {
2862+
cbm_store_t *s = setup_cypher_store();
2863+
cbm_cypher_result_t r = {0};
2864+
2865+
int rc = cbm_cypher_execute(
2866+
s, "MATCH (a) OPTIONAL MATCH (a)-[:CALLS]->(b) RETURN a.qualified_name, count(b)", "test",
2867+
0, &r);
2868+
2869+
ASSERT_EQ(rc, 0);
2870+
ASSERT_TRUE(r.error == NULL);
2871+
ASSERT_GT(r.row_count, 0);
2872+
2873+
cbm_cypher_result_free(&r);
2874+
cbm_store_close(s);
2875+
PASS();
2876+
}
2877+
28302878
/* ══════════════════════════════════════════════════════════════════ */
28312879

28322880
SUITE(cypher) {
@@ -2859,6 +2907,8 @@ SUITE(cypher) {
28592907
RUN_TEST(cypher_parse_inline_props);
28602908
RUN_TEST(cypher_parse_error);
28612909
/* Execution */
2910+
RUN_TEST(cypher_exec_deadline_aborts_runaway_query_issue601);
2911+
RUN_TEST(cypher_exec_deadline_allows_normal_query_issue601);
28622912
RUN_TEST(cypher_exec_match_all_functions);
28632913
RUN_TEST(cypher_issue240_labels_function);
28642914
RUN_TEST(cypher_issue237_distinct_order_limit);

0 commit comments

Comments
 (0)