diff --git a/src/runtime/api/zlib.classes.ts b/src/runtime/api/zlib.classes.ts index 37f80fc47638..38c57837d127 100644 --- a/src/runtime/api/zlib.classes.ts +++ b/src/runtime/api/zlib.classes.ts @@ -10,7 +10,7 @@ function generate(name: string) { estimatedSize: true, klass: {}, JSType: "0b11101110", - values: ["writeCallback", "errorCallback", "dictionary", "pendingInput", "pendingOutput"], + values: ["writeCallback", "errorCallback", "dictionary", "pendingInput", "pendingOutput", "writeState"], proto: { init: { fn: "init" }, diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index a3d4e02a28a6..da1971d132bb 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -237,8 +237,9 @@ pub(crate) trait CompressionStreamImpl: Sized + Taskable + 'static { }; // SAFETY: `write_result` points at a 2-element `u32[]` owned by JS // (set in each impl's `init()`); both indices are in-bounds and the - // backing buffer is kept alive by `this._writeState` / - // `_handle[owner_symbol]`. + // backing buffer is both pinned (so `.transfer()` can't free it) and + // held in the wrapper's GC-visited `writeState` slot (so it can't be + // collected) for the handle's lifetime — see each `init()`. let (r1, r0) = unsafe { (&mut *write_result.add(1), &mut *write_result) }; self.stream().with_mut(|s| s.update_write_result(r1, r0)); } @@ -1001,7 +1002,7 @@ macro_rules! __impl_compression_stream { /// `generate-classes.ts` for the `values:` list in `zlib.classes.ts`. #[allow(unused)] pub(crate) mod js { - ::bun_jsc::codegen_cached_accessors!($type_name; writeCallback, errorCallback, dictionary, pendingInput, pendingOutput); + ::bun_jsc::codegen_cached_accessors!($type_name; writeCallback, errorCallback, dictionary, pendingInput, pendingOutput, writeState); } impl $crate::node::node_zlib_binding::CompressionContext for $ctx { diff --git a/src/runtime/node/zlib/NativeBrotli.rs b/src/runtime/node/zlib/NativeBrotli.rs index 8bfe10b4c107..387680763e69 100644 --- a/src/runtime/node/zlib/NativeBrotli.rs +++ b/src/runtime/node/zlib/NativeBrotli.rs @@ -86,7 +86,7 @@ mod _impl { pub global_this: bun_ptr::BackRef, pub stream: JsCell, /// Points into a JS `Uint32Array` (`this._writeState`). Kept alive because - /// the JS object is tied to the native handle as `_handle[owner_symbol]`. + /// `init()` stores the view in the wrapper's GC-visited `writeState` slot. pub write_result: Cell>, pub poll_ref: JsCell, // TODO(port): Strong on m_ctx self-ref → JsRef per PORTING.md §JSC (Strong back-ref to own wrapper leaks) @@ -188,11 +188,10 @@ mod _impl { .throw()); } - // this does not get gc'd because it is stored in the JS object's - // `this._writeState`. and the JS object is tied to the native handle - // as `_handle[owner_symbol]`. // `flush_write_result` writes two u32s through this pointer, so the - // caller-supplied array must hold at least 2 elements. + // caller-supplied array must hold at least 2 elements. The backing + // store is pinned and rooted (GC-visited `writeState` slot) below, + // so the cached pointer can't dangle. let write_result_value = arguments.ptr[1]; let Some(mut write_result_buf) = write_result_value.as_array_buffer(global_this) else { return Err(global_this.throw_invalid_argument_type_value( @@ -217,7 +216,6 @@ mod _impl { ) .throw()); } - let write_result = write_result_slice.as_mut_ptr(); let write_callback = validators::validate_function(global_this, "writeCallback", arguments.ptr[2])?; @@ -240,7 +238,20 @@ mod _impl { )); } - self.write_result.set(Some(write_result)); + // Pin the `_writeState` backing store so `transfer()` can't free it + // under the raw pointer cached below (written through on every + // async-write completion by `flush_write_result`). Pinning a small + // FastTypedArray relocates its storage, so read the pointer *after*. + // The GC-visited `writeState` slot (zlib.classes.ts `values:`) keeps + // the view alive for the wrapper's lifetime — the pin only blocks + // detach, not collection — so the cached pointer can never dangle. + let Some(mut write_result_buf) = write_result_value.as_pinned_arraybuffer(global_this) + else { + return Err(global_this.throw_out_of_memory()); + }; + js::write_state_set_cached(this_value, global_this, write_result_value); + self.write_result + .set(Some(write_result_buf.as_u32().as_mut_ptr())); js::write_callback_set_cached( this_value, diff --git a/src/runtime/node/zlib/NativeZlib.rs b/src/runtime/node/zlib/NativeZlib.rs index eef9eeb4a138..d07f49162a2c 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -141,9 +141,10 @@ mod _impl { validators::validate_int32(global, arguments.ptr[2], "memLevel", None, None)?; let strategy = validators::validate_int32(global, arguments.ptr[3], "strategy", None, None)?; - // this does not get gc'd because it is stored in the JS object's `this._writeState`. and the JS object is tied to the native handle as `_handle[owner_symbol]`. // `flush_write_result` writes two u32s through this pointer, so the - // caller-supplied array must hold at least 2 elements. + // caller-supplied array must hold at least 2 elements. The backing + // store is pinned and rooted (GC-visited `writeState` slot) below, + // so the cached pointer can't dangle. let write_result_value = arguments.ptr[4]; let Some(mut write_result_buf) = write_result_value.as_array_buffer(global) else { return Err(global.throw_invalid_argument_type_value( @@ -168,7 +169,6 @@ mod _impl { ) .throw()); } - let write_result = write_result_slice.as_mut_ptr(); let write_callback = validators::validate_function(global, "writeCallback", arguments.ptr[5])?; // Bind the ArrayBuffer view to a local so the borrowed byte_slice() outlives @@ -191,7 +191,20 @@ mod _impl { Some(dictionary_buf.byte_slice()) }; - self.write_result.set(Some(write_result)); + // Pin the `_writeState` backing store so `transfer()` can't free it + // under the raw pointer cached below (written through on every + // async-write completion by `flush_write_result`). Pinning a small + // FastTypedArray relocates its storage, so read the pointer *after*. + // The GC-visited `writeState` slot (zlib.classes.ts `values:`) keeps + // the view alive for the wrapper's lifetime — the pin only blocks + // detach, not collection — so the cached pointer can never dangle. + let Some(mut write_result_buf) = write_result_value.as_pinned_arraybuffer(global) + else { + return Err(global.throw_out_of_memory()); + }; + js::write_state_set_cached(this_value, global, write_result_value); + self.write_result + .set(Some(write_result_buf.as_u32().as_mut_ptr())); js::write_callback_set_cached( this_value, global, diff --git a/src/runtime/node/zlib/NativeZstd.rs b/src/runtime/node/zlib/NativeZstd.rs index d3a7aefd4c42..8941702fb7c4 100644 --- a/src/runtime/node/zlib/NativeZstd.rs +++ b/src/runtime/node/zlib/NativeZstd.rs @@ -171,8 +171,6 @@ mod _impl { ) .throw()); } - self.write_result.set(Some(write_state_slice.as_mut_ptr())); - let write_js_callback = validators::validate_function(global, "processCallback", process_callback_value)?; // js.writeCallbackSetCached — codegen'd cached-property setter on the C++ wrapper. @@ -192,6 +190,22 @@ mod _impl { )?); } + // Pin the `_writeState` backing store so `transfer()` can't free it + // under the raw pointer cached below (written through on every + // async-write completion by `flush_write_result`). Pinning a small + // FastTypedArray relocates its storage, so read the pointer *after*. + // The GC-visited `writeState` slot (zlib.classes.ts `values:`) keeps + // the view alive for the wrapper's lifetime — the pin only blocks + // detach, not collection — so the cached pointer can never dangle. + // Placed after the argument validators (mirroring NativeZlib and + // NativeBrotli) so their error paths leave nothing pinned. + let Some(mut write_state) = write_state_value.as_pinned_arraybuffer(global) else { + return Err(global.throw_out_of_memory()); + }; + js::write_state_set_cached(this_value, global, write_state_value); + self.write_result + .set(Some(write_state.as_u32().as_mut_ptr())); + let err = self.stream.with_mut(|s| s.init(pledged_src_size)); if err.is_error() { CompressionStream::::emit_error(self, global, this_value, err); diff --git a/test/js/node/zlib/zlib-writestate-transfer.test.js b/test/js/node/zlib/zlib-writestate-transfer.test.js new file mode 100644 index 000000000000..3335048b54e1 --- /dev/null +++ b/test/js/node/zlib/zlib-writestate-transfer.test.js @@ -0,0 +1,46 @@ +// The native zlib/brotli/zstd handle caches a raw pointer into `_writeState`'s +// backing store at init() and writes two u32s through it on every async-write +// completion. Detaching that backing store while a write is in flight must not +// leave the native side writing through a stale pointer — the buffer is pinned +// (and kept in a GC-visited slot) for the handle's lifetime so transfer() +// becomes a copy and the stream still produces correct output. + +import { describe, expect, it } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +describe("_writeState buffer lifetime", () => { + // Run in a subprocess because an unpinned build corrupts stream bookkeeping + // (or faults) rather than throwing. + it.concurrent.each([ + ["Inflate", "deflateSync", "createInflate"], + ["BrotliDecompress", "brotliCompressSync", "createBrotliDecompress"], + ["ZstdDecompress", "zstdCompressSync", "createZstdDecompress"], + ])("%s decompresses correctly when _writeState.buffer is transferred mid-write", async (_, compress, create) => { + const script = ` + const z = require("zlib"); + const raw = Buffer.alloc(65536, 0x41); + const d = z.${compress}(raw); + const s = z.${create}({ chunkSize: 65536 }); + const out = []; + s.on("error", e => { console.log("err:" + e.message); process.exit(1); }); + s.on("data", c => out.push(c)); + s.on("end", () => { + const r = Buffer.concat(out); + if (r.length === 65536 && r.equals(raw)) console.log("OK"); + else console.log("BAD len=" + r.length); + }); + s.write(d); + s._writeState.buffer.transfer(0); + s.end(); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("OK"); + expect(exitCode).toBe(0); + }); +});