Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/runtime/api/zlib.classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down
7 changes: 4 additions & 3 deletions src/runtime/node/node_zlib_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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 {
Expand Down
25 changes: 18 additions & 7 deletions src/runtime/node/zlib/NativeBrotli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ mod _impl {
pub global_this: bun_ptr::BackRef<JSGlobalObject>,
pub stream: JsCell<Context>,
/// 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.
Comment thread
robobun marked this conversation as resolved.
pub write_result: Cell<Option<*mut u32>>,
pub poll_ref: JsCell<CountedKeepAlive>,
// TODO(port): Strong on m_ctx self-ref → JsRef per PORTING.md §JSC (Strong back-ref to own wrapper leaks)
Expand Down Expand Up @@ -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(
Expand All @@ -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])?;

Expand All @@ -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,
Expand Down
21 changes: 17 additions & 4 deletions src/runtime/node/zlib/NativeZlib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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,
Expand Down
18 changes: 16 additions & 2 deletions src/runtime/node/zlib/NativeZstd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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::<Self>::emit_error(self, global, this_value, err);
Expand Down
46 changes: 46 additions & 0 deletions test/js/node/zlib/zlib-writestate-transfer.test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading