doc gc bugs
This commit is contained in:
47
CLAUDE.md
47
CLAUDE.md
@@ -124,16 +124,19 @@ This project uses a **copying garbage collector**. ANY JS allocation (`JS_NewObj
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(obj, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, obj.val, "x", JS_NewInt32(js, 42));
|
||||
JS_SetPropertyStr(js, obj.val, "name", JS_NewString(js, "hello"));
|
||||
JSValue name = JS_NewString(js, "hello");
|
||||
JS_SetPropertyStr(js, obj.val, "name", name);
|
||||
JS_RETURN(obj.val);
|
||||
```
|
||||
|
||||
**Pattern — array with loop:**
|
||||
**Pattern — array with loop (declare root BEFORE the loop):**
|
||||
```c
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(arr, JS_NewArray(js));
|
||||
JSGCRef item = { .val = JS_NULL, .prev = NULL };
|
||||
JS_PushGCRef(js, &item);
|
||||
for (int i = 0; i < count; i++) {
|
||||
JS_ROOT(item, JS_NewObject(js));
|
||||
item.val = JS_NewObject(js);
|
||||
JS_SetPropertyStr(js, item.val, "v", JS_NewInt32(js, i));
|
||||
JS_SetPropertyNumber(js, arr.val, i, item.val);
|
||||
}
|
||||
@@ -142,18 +145,28 @@ JS_RETURN(arr.val);
|
||||
|
||||
**Rules:**
|
||||
- Access rooted values via `.val` (e.g., `obj.val`, not `obj`)
|
||||
- NEVER put `JS_ROOT` inside a loop — it pushes the same stack address twice, corrupting the GC chain
|
||||
- Error returns before `JS_FRAME` use plain `return`
|
||||
- Error returns after `JS_FRAME` must use `JS_RETURN_EX()` or `JS_RETURN_NULL()`
|
||||
- When calling a helper that itself returns a JSValue, that return value is safe to pass directly into `JS_SetPropertyStr` — no need to root temporaries that aren't stored in a local
|
||||
|
||||
**Common mistake — UNSAFE (will crash under GC pressure):**
|
||||
**CRITICAL — C argument evaluation order bug:**
|
||||
|
||||
Allocating functions (`JS_NewString`, `JS_NewFloat64`, `js_new_blob_stoned_copy`, etc.) used as arguments to `JS_SetPropertyStr` can crash because C evaluates arguments in unspecified order. The compiler may read `obj.val` BEFORE the allocating call, then GC moves the object, leaving a stale pointer.
|
||||
|
||||
```c
|
||||
JSValue obj = JS_NewObject(js); // NOT rooted
|
||||
JS_SetPropertyStr(js, obj, "pixels", js_new_blob_stoned_copy(js, data, len));
|
||||
// ^^^ blob allocation can GC, invalidating obj
|
||||
return obj; // obj may be a dangling pointer
|
||||
// UNSAFE — intermittent crash:
|
||||
JS_SetPropertyStr(js, obj.val, "format", JS_NewString(js, "rgba32"));
|
||||
JS_SetPropertyStr(js, obj.val, "pixels", js_new_blob_stoned_copy(js, data, len));
|
||||
|
||||
// SAFE — separate the allocation:
|
||||
JSValue fmt = JS_NewString(js, "rgba32");
|
||||
JS_SetPropertyStr(js, obj.val, "format", fmt);
|
||||
JSValue pixels = js_new_blob_stoned_copy(js, data, len);
|
||||
JS_SetPropertyStr(js, obj.val, "pixels", pixels);
|
||||
```
|
||||
|
||||
`JS_NewInt32`, `JS_NewUint32`, and `JS_NewBool` do NOT allocate and are safe inline.
|
||||
|
||||
See `docs/c-modules.md` for the full GC safety reference.
|
||||
|
||||
## Project Layout
|
||||
@@ -167,17 +180,19 @@ See `docs/c-modules.md` for the full GC safety reference.
|
||||
|
||||
## Package Management (Shop CLI)
|
||||
|
||||
When running locally with `./cell --dev`, these commands manage packages:
|
||||
**Two shops:** `cell <cmd>` uses the global shop at `~/.cell/packages/`. `cell --dev <cmd>` uses the local shop at `.cell/packages/`. Linked packages (via `cell link`) are symlinked into the shop — edit the source directory directly.
|
||||
|
||||
```
|
||||
./cell --dev add <path> # add a package (local path or remote)
|
||||
./cell --dev remove <path> # remove a package (cleans lock, symlink, dylibs)
|
||||
./cell --dev build <path> # build C modules for a package
|
||||
./cell --dev test package <path> # run tests for a package
|
||||
./cell --dev list # list installed packages
|
||||
cell add <path> # add a package (local path or remote)
|
||||
cell remove <path> # remove a package (cleans lock, symlink, dylibs)
|
||||
cell build <path> # build C modules for a package
|
||||
cell build <path> --force # force rebuild (ignore stat cache)
|
||||
cell test package <path> # run tests for a package
|
||||
cell list # list installed packages
|
||||
cell link # list linked packages
|
||||
```
|
||||
|
||||
Local paths are symlinked into `.cell/packages/`. The build step compiles C files to content-addressed dylibs in `~/.cell/build/<hash>` and writes a per-package manifest so the runtime can find them. C files in `src/` are support files linked into module dylibs, not standalone modules.
|
||||
The build step compiles C files to content-addressed dylibs in `~/.cell/build/<hash>` and writes a per-package manifest so the runtime can find them. C files in `src/` are support files linked into module dylibs, not standalone modules.
|
||||
|
||||
## Debugging Compiler Issues
|
||||
|
||||
|
||||
@@ -453,6 +453,53 @@ JSC_CCALL(mymod_make,
|
||||
)
|
||||
```
|
||||
|
||||
### C Argument Evaluation Order (critical)
|
||||
|
||||
In C, the **order of evaluation of function arguments is unspecified**. This interacts with the copying GC to create intermittent crashes that are extremely difficult to diagnose.
|
||||
|
||||
```c
|
||||
// UNSAFE — crashes intermittently:
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(obj, JS_NewObject(js));
|
||||
JS_SetPropertyStr(js, obj.val, "format", JS_NewString(js, "rgba32"));
|
||||
// ^^^^^^^ may be evaluated BEFORE JS_NewString runs
|
||||
// If JS_NewString triggers GC, the already-read obj.val is a dangling pointer.
|
||||
```
|
||||
|
||||
The compiler is free to evaluate `obj.val` into a register, *then* call `JS_NewString`. If `JS_NewString` triggers GC, the object moves to a new address. The rooted `obj` is updated by GC, but the **register copy** is not — it still holds the old address. `JS_SetPropertyStr` then writes to freed memory.
|
||||
|
||||
**Fix:** always separate the allocating call into a local variable:
|
||||
|
||||
```c
|
||||
// SAFE:
|
||||
JS_FRAME(js);
|
||||
JS_ROOT(obj, JS_NewObject(js));
|
||||
JSValue fmt = JS_NewString(js, "rgba32");
|
||||
JS_SetPropertyStr(js, obj.val, "format", fmt);
|
||||
// obj.val is read AFTER JS_NewString completes — guaranteed correct.
|
||||
```
|
||||
|
||||
This applies to **any** allocating function used as an argument when another argument references a rooted `.val`:
|
||||
|
||||
```c
|
||||
// ALL of these are UNSAFE:
|
||||
JS_SetPropertyStr(js, obj.val, "pixels", js_new_blob_stoned_copy(js, data, len));
|
||||
JS_SetPropertyStr(js, obj.val, "x", JS_NewFloat64(js, 3.14));
|
||||
JS_SetPropertyStr(js, obj.val, "name", JS_NewString(js, name));
|
||||
|
||||
// SAFE versions — separate the allocation:
|
||||
JSValue pixels = js_new_blob_stoned_copy(js, data, len);
|
||||
JS_SetPropertyStr(js, obj.val, "pixels", pixels);
|
||||
JSValue x = JS_NewFloat64(js, 3.14);
|
||||
JS_SetPropertyStr(js, obj.val, "x", x);
|
||||
JSValue s = JS_NewString(js, name);
|
||||
JS_SetPropertyStr(js, obj.val, "name", s);
|
||||
```
|
||||
|
||||
**Functions that allocate** (must be separated): `JS_NewString`, `JS_NewFloat64`, `JS_NewInt64`, `JS_NewObject`, `JS_NewArray`, `JS_NewCFunction`, `js_new_blob_stoned_copy`
|
||||
|
||||
**Functions that do NOT allocate** (safe inline): `JS_NewInt32`, `JS_NewUint32`, `JS_NewBool`, `JS_NULL`, `JS_TRUE`, `JS_FALSE`
|
||||
|
||||
### Macros
|
||||
|
||||
| Macro | Purpose |
|
||||
|
||||
Reference in New Issue
Block a user