add guards for root cycles

This commit is contained in:
2026-02-18 18:27:12 -06:00
parent 94fe47b472
commit dc70a15981
2 changed files with 9 additions and 2 deletions

View File

@@ -410,13 +410,14 @@ JS_SetPropertyStr(js, result.val, "pixels", js_new_blob_stoned_copy(js, data, le
JS_RETURN(result.val); JS_RETURN(result.val);
``` ```
**Array with loop** — root both the array and each element created in the loop: **Array with loop** — root the element variable *before* the loop, then reassign `.val` each iteration:
```c ```c
JS_FRAME(js); JS_FRAME(js);
JS_ROOT(arr, JS_NewArray(js)); JS_ROOT(arr, JS_NewArray(js));
JS_ROOT(item, JS_NULL);
for (int i = 0; i < count; i++) { for (int i = 0; i < count; i++) {
JS_ROOT(item, JS_NewObject(js)); item.val = JS_NewObject(js);
JS_SetPropertyStr(js, item.val, "index", JS_NewInt32(js, i)); JS_SetPropertyStr(js, item.val, "index", JS_NewInt32(js, i));
JS_SetPropertyStr(js, item.val, "data", js_new_blob_stoned_copy(js, ptr, sz)); JS_SetPropertyStr(js, item.val, "data", js_new_blob_stoned_copy(js, ptr, sz));
JS_SetPropertyNumber(js, arr.val, i, item.val); JS_SetPropertyNumber(js, arr.val, i, item.val);
@@ -424,6 +425,8 @@ for (int i = 0; i < count; i++) {
JS_RETURN(arr.val); JS_RETURN(arr.val);
``` ```
**WARNING — NEVER put `JS_ROOT` inside a loop.** `JS_ROOT` declares a `JSGCRef` local and calls `JS_PushGCRef(&name)`, which pushes its address onto a linked list. Inside a loop the compiler reuses the same stack address, so on iteration 2+ the list becomes self-referential (`ref->prev == ref`). When GC triggers it walks the chain and **hangs forever**. This bug is intermittent — it only manifests when GC happens to run during the loop — making it very hard to reproduce.
**Nested objects** — root every object that persists across an allocating call: **Nested objects** — root every object that persists across an allocating call:
```c ```c

View File

@@ -150,6 +150,7 @@ int JS_IsPretext (JSValue v) {
} }
JSValue *JS_PushGCRef (JSContext *ctx, JSGCRef *ref) { JSValue *JS_PushGCRef (JSContext *ctx, JSGCRef *ref) {
assert(ref != ctx->top_gc_ref && "JS_ROOT used in a loop — same address pushed twice");
ref->prev = ctx->top_gc_ref; ref->prev = ctx->top_gc_ref;
ctx->top_gc_ref = ref; ctx->top_gc_ref = ref;
ref->val = JS_NULL; ref->val = JS_NULL;
@@ -157,11 +158,13 @@ JSValue *JS_PushGCRef (JSContext *ctx, JSGCRef *ref) {
} }
JSValue JS_PopGCRef (JSContext *ctx, JSGCRef *ref) { JSValue JS_PopGCRef (JSContext *ctx, JSGCRef *ref) {
assert(ctx->top_gc_ref == ref && "JS_PopGCRef: not popping top of stack — mismatched push/pop");
ctx->top_gc_ref = ref->prev; ctx->top_gc_ref = ref->prev;
return ref->val; return ref->val;
} }
JSValue *JS_AddGCRef (JSContext *ctx, JSGCRef *ref) { JSValue *JS_AddGCRef (JSContext *ctx, JSGCRef *ref) {
assert(ref != ctx->last_gc_ref && "JS_AddGCRef: same address added twice — cycle in GC ref list");
ref->prev = ctx->last_gc_ref; ref->prev = ctx->last_gc_ref;
ctx->last_gc_ref = ref; ctx->last_gc_ref = ref;
ref->val = JS_NULL; ref->val = JS_NULL;
@@ -193,6 +196,7 @@ JSLocalRef *JS_GetLocalFrame (JSContext *ctx) {
} }
void JS_PushLocalRef (JSContext *ctx, JSLocalRef *ref) { void JS_PushLocalRef (JSContext *ctx, JSLocalRef *ref) {
assert(ref != ctx->top_local_ref && "JS_LOCAL used in a loop — same address pushed twice");
ref->prev = ctx->top_local_ref; ref->prev = ctx->top_local_ref;
ctx->top_local_ref = ref; ctx->top_local_ref = ref;
} }