From 4a0a28afc4b460cf4958c033e3991d29e521d60d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:55:50 +0000 Subject: [PATCH] Unpin ArrayBuffers without classInfo() so blob finalizers are safe during GC sweep Blob stores for Bun.file(bufferPath) and S3Client.file(bufferKey) keep the pinned path buffer until the store drops, which happens in the JS wrapper's finalizer during the sweep phase. JSC__JSValue__unpinArrayBuffer re-derived the JSC::ArrayBuffer* with dynamicDowncast and possiblySharedBuffer(), both of which call JSCell::classInfo() in debug builds and trip the validateIsNotSweeping assertion (JSCell.cpp:179). Dispatch on the cell's JSType with static_cast instead, reading a DataView's buffer field directly and a wasteful view's buffer out of the butterfly indexing header, like JSC::Weak::get() does for finalizer contexts. A view whose mode has no ArrayBuffer (Fast/Oversize, which pinStorage reports as None/Held) stays a no-op, matching the previous hasArrayBuffer() check. Release builds already compiled the old downcasts to the same type checks, so behavior there is unchanged. --- src/jsc/bindings/bindings.cpp | 19 ++++++++--- test/js/bun/s3/s3-stream-error-gc.test.ts | 40 +++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 983a348f73a5..6ab21f203d43 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -3538,12 +3538,23 @@ CPP_DECL uint8_t JSC__JSValue__pinArrayBuffer(JSC::EncodedJSValue v) // Only for a value `pinStorage` answered `Pinned` for: that buffer still exists (pinned buffers are not detached). CPP_DECL void JSC__JSValue__unpinArrayBuffer(JSC::EncodedJSValue v) { + // Reached from finalizers during GC sweep, where classInfo() (and so any + // dynamicDowncast) is forbidden; dispatch on JSType like JSC::Weak::get(). auto value = JSC::JSValue::decode(v); + if (!value.isCell()) + return; + JSC::JSCell* cell = value.asCell(); + JSC::JSType type = cell->type(); JSC::ArrayBuffer* buf = nullptr; - if (auto* jb = dynamicDowncast(value)) - buf = jb->impl(); - else if (auto* view = dynamicDowncast(value); view && view->hasArrayBuffer()) - buf = view->possiblySharedBuffer(); + if (type == JSC::ArrayBufferType) + buf = static_cast(cell)->impl(); + else if (type == JSC::DataViewType) + buf = static_cast(cell)->possiblySharedBuffer(); + else if (JSC::isTypedArrayType(type)) { + auto* view = static_cast(cell); + if (JSC::isWastefulTypedArray(view->mode())) + buf = view->butterfly()->indexingHeader()->arrayBuffer(); + } if (buf && !buf->isShared()) buf->unpin(); } diff --git a/test/js/bun/s3/s3-stream-error-gc.test.ts b/test/js/bun/s3/s3-stream-error-gc.test.ts index ea47df3c706d..404fe1778c10 100644 --- a/test/js/bun/s3/s3-stream-error-gc.test.ts +++ b/test/js/bun/s3/s3-stream-error-gc.test.ts @@ -1,6 +1,46 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, normalizeBunSnapshot } from "harness"; +test.concurrent("collecting file blobs with Buffer paths does not crash during GC sweep", async () => { + // The S3/file blob store keeps the pinned path buffer until the wrapper is + // finalized inside the GC sweep; releasing the pin must not reach + // JSCell::classInfo() there (validateIsNotSweeping assert in debug builds). + const fixture = ` + const enc = new TextEncoder(); + for (let i = 0; i < 50; i++) { + new Bun.S3Client({}).file(Buffer.from("key-" + i)); + new Bun.S3Client({}).file(new DataView(enc.encode("dv-key-" + i).buffer)); + new Bun.S3Client({}).file(enc.encode("uint8-key-" + i)); + Bun.file(Buffer.from("/tmp/buffer-path-" + i)); + Bun.file(enc.encode("/tmp/uint8-path-" + i)); + Bun.file(enc.encode("/tmp/enc-path-" + i).buffer); + Bun.gc(true); + } + console.log("ok"); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ + stdout: normalizeBunSnapshot(stdout), + stderr: normalizeBunSnapshot(stderr), + exitCode, + }).toMatchInlineSnapshot(` + { + "exitCode": 0, + "stderr": "", + "stdout": "ok", + } + `); +}); + test("S3 stream error parked before consumption survives GC", async () => { const fixture = ` const stream = Bun.S3Client.file("some-key").stream();