Skip to content

Commit

Permalink
+5 passing node:zlib tests (#15944)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jarred-Sumner
Jarred-Sumner authored Dec 23, 2024
1 parent a6ad3b9 commit 4f8a6b3
Show file tree
Hide file tree
Showing 12 changed files with 796 additions and 199 deletions.
40 changes: 9 additions & 31 deletions src/bun.js/api/zlib.classes.ts
View file
Original file line number Diff line number Diff line change
@@ -1,40 +1,16 @@
import { define } from "../../codegen/class-definitions";

export default [
define({
name: "NativeZlib",
function generate(name: string) {
return define({
name,
construct: true,
noConstructor: false,
wantsThis: true,
finalize: true,
configurable: false,
// estimatedSize: true,
klass: {},
JSType: "0b11101110",
values: ["callback"],

proto: {
init: { fn: "init" },
write: { fn: "write" },
writeSync: { fn: "writeSync" },
params: { fn: "params" },
reset: { fn: "reset" },
close: { fn: "close" },
onerror: { setter: "setOnError", getter: "getOnError" },
},
}),

define({
name: "NativeBrotli",
construct: true,
noConstructor: false,
wantsThis: true,
finalize: true,
configurable: false,
estimatedSize: true,
klass: {},
JSType: "0b11101110",
values: ["callback"],
values: ["writeCallback", "errorCallback"],

proto: {
init: { fn: "init" },
Expand All @@ -43,7 +19,9 @@ export default [
params: { fn: "params" },
reset: { fn: "reset" },
close: { fn: "close" },
onerror: { setter: "setOnError", getter: "getOnError" },
onerror: { setter: "setOnError", this: true, getter: "getOnError" },
},
}),
];
});
}

export default [generate("NativeZlib"), generate("NativeBrotli")];
85 changes: 49 additions & 36 deletions src/bun.js/node/node_zlib_binding.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const string = bun.string;
const Output = bun.Output;
const ZigString = JSC.ZigString;
const validators = @import("./util/validators.zig");
const debug = bun.Output.scoped(.zlib, true);

pub fn crc32(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
const arguments = callframe.arguments_old(2).ptr;
Expand Down Expand Up @@ -71,6 +72,8 @@ pub fn CompressionStream(comptime T: type) type {
var in: ?[]const u8 = null;
var out: ?[]u8 = null;

const this_value = callframe.this();

bun.assert(!arguments[0].isUndefined()); // must provide flush value
flush = arguments[0].toU32();
_ = std.meta.intToEnum(bun.zlib.FlushValue, flush) catch bun.assert(false); // Invalid flush value
Expand Down Expand Up @@ -102,7 +105,9 @@ pub fn CompressionStream(comptime T: type) type {
this.stream.setBuffers(in, out);
this.stream.setFlush(@intCast(flush));

//
// Only create the strong handle when we have a pending write
// And make sure to clear it when we are done.
this.this_value.set(globalThis, this_value);

const vm = globalThis.bunVM();
this.task = .{ .callback = &AsyncJob.runTask };
Expand Down Expand Up @@ -136,13 +141,23 @@ pub fn CompressionStream(comptime T: type) type {

this.write_in_progress = false;

if (!(this.checkError(globalThis) catch return globalThis.reportActiveExceptionAsUnhandled(error.JSError))) {
// Clear the strong handle before we call any callbacks.
const this_value = this.this_value.trySwap() orelse {
debug("this_value is null in runFromJSThread", .{});
return;
};

this_value.ensureStillAlive();

if (!(this.checkError(globalThis, this_value) catch return globalThis.reportActiveExceptionAsUnhandled(error.JSError))) {
return;
}

this.stream.updateWriteResult(&this.write_result.?[1], &this.write_result.?[0]);
this_value.ensureStillAlive();

_ = this.write_callback.get().?.call(globalThis, this.this_value.get().?, &.{}) catch |err| globalThis.reportActiveExceptionAsUnhandled(err);
const write_callback: JSC.JSValue = T.writeCallbackGetCached(this_value).?;
_ = write_callback.call(globalThis, this_value, &.{}) catch |err| globalThis.reportActiveExceptionAsUnhandled(err);

if (this.pending_close) _ = this._close();
}
Expand Down Expand Up @@ -192,11 +207,10 @@ pub fn CompressionStream(comptime T: type) type {

this.stream.setBuffers(in, out);
this.stream.setFlush(@intCast(flush));

//
const this_value = callframe.this();

this.stream.doWork();
if (try this.checkError(globalThis)) {
if (try this.checkError(globalThis, this_value)) {
this.stream.updateWriteResult(&this.write_result.?[1], &this.write_result.?[0]);
this.write_in_progress = false;
}
Expand All @@ -206,11 +220,9 @@ pub fn CompressionStream(comptime T: type) type {
}

pub fn reset(this: *T, globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
_ = callframe;

const err = this.stream.reset();
if (err.isError()) {
try this.emitError(globalThis, err);
try this.emitError(globalThis, callframe.this(), err);
}
return .undefined;
}
Expand All @@ -233,34 +245,34 @@ pub fn CompressionStream(comptime T: type) type {
this.stream.close();
}

pub fn setOnError(this: *T, globalThis: *JSC.JSGlobalObject, value: JSC.JSValue) bool {
pub fn setOnError(_: *T, this_value: JSC.JSValue, globalObject: *JSC.JSGlobalObject, value: JSC.JSValue) bool {
if (value.isFunction()) {
this.onerror_value.set(globalThis, value);
T.errorCallbackSetCached(this_value, globalObject, value);
}
return true;
}

pub fn getOnError(this: *T, globalThis: *JSC.JSGlobalObject) JSC.JSValue {
_ = globalThis;
return this.onerror_value.get() orelse .undefined;
pub fn getOnError(_: *T, this_value: JSC.JSValue, _: *JSC.JSGlobalObject) JSC.JSValue {
return T.errorCallbackGetCached(this_value) orelse .undefined;
}

/// returns true if no error was detected/emitted
fn checkError(this: *T, globalThis: *JSC.JSGlobalObject) !bool {
fn checkError(this: *T, globalThis: *JSC.JSGlobalObject, this_value: JSC.JSValue) !bool {
const err = this.stream.getErrorInfo();
if (!err.isError()) return true;
try this.emitError(globalThis, err);
try this.emitError(globalThis, this_value, err);
return false;
}

fn emitError(this: *T, globalThis: *JSC.JSGlobalObject, err_: Error) !void {
fn emitError(this: *T, globalThis: *JSC.JSGlobalObject, this_value: JSC.JSValue, err_: Error) !void {
var msg_str = bun.String.createFormat("{s}", .{std.mem.sliceTo(err_.msg, 0) orelse ""}) catch bun.outOfMemory();
const msg_value = msg_str.transferToJS(globalThis);
const err_value = JSC.jsNumber(err_.err);
var code_str = bun.String.createFormat("{s}", .{std.mem.sliceTo(err_.code, 0) orelse ""}) catch bun.outOfMemory();
const code_value = code_str.transferToJS(globalThis);

_ = try this.onerror_value.get().?.call(globalThis, this.this_value.get().?, &.{ msg_value, err_value, code_value });
const callback: JSC.JSValue = T.errorCallbackGetCached(this_value) orelse Output.panic("Assertion failure: cachedErrorCallback is null in node:zlib binding", .{});
_ = try callback.call(globalThis, this_value, &.{ msg_value, err_value, code_value });

this.write_in_progress = false;
if (this.pending_close) _ = this._close();
Expand Down Expand Up @@ -307,8 +319,6 @@ pub const SNativeZlib = struct {
globalThis: *JSC.JSGlobalObject,
stream: ZlibContext = .{},
write_result: ?[*]u32 = null,
write_callback: JSC.Strong = .{},
onerror_value: JSC.Strong = .{},
poll_ref: CountedKeepAlive = .{},
this_value: JSC.Strong = .{},
write_in_progress: bool = false,
Expand Down Expand Up @@ -341,11 +351,10 @@ pub const SNativeZlib = struct {
}

//// adding this didnt help much but leaving it here to compare the number with later
// pub fn estimatedSize(this: *const SNativeZlib) usize {
// _ = this;
// const internal_state_size = 3309; // @sizeOf(@cImport(@cInclude("deflate.h")).internal_state) @ cloudflare/zlib @ 92530568d2c128b4432467b76a3b54d93d6350bd
// return @sizeOf(SNativeZlib) + internal_state_size;
// }
pub fn estimatedSize(_: *const SNativeZlib) usize {
const internal_state_size = 3309; // @sizeOf(@cImport(@cInclude("deflate.h")).internal_state) @ cloudflare/zlib @ 92530568d2c128b4432467b76a3b54d93d6350bd
return @sizeOf(SNativeZlib) + internal_state_size;
}

pub fn init(this: *SNativeZlib, globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
const arguments = callframe.argumentsUndef(7).slice();
Expand All @@ -364,7 +373,7 @@ pub const SNativeZlib = struct {
const dictionary = if (arguments[6].isUndefined()) null else arguments[6].asArrayBuffer(globalThis).?.byteSlice();

this.write_result = writeResult;
this.write_callback.set(globalThis, writeCallback);
SNativeZlib.writeCallbackSetCached(callframe.this(), globalThis, writeCallback);

this.stream.init(level, windowBits, memLevel, strategy, dictionary);

Expand All @@ -383,15 +392,15 @@ pub const SNativeZlib = struct {

const err = this.stream.setParams(level, strategy);
if (err.isError()) {
try this.emitError(globalThis, err);
try this.emitError(globalThis, callframe.this(), err);
}
return .undefined;
}

pub fn deinit(this: *@This()) void {
this.write_callback.deinit();
this.onerror_value.deinit();
this.this_value.deinit();
this.poll_ref.deinit();
this.stream.close();
this.destroy();
}
};
Expand Down Expand Up @@ -662,16 +671,14 @@ pub const NativeBrotli = JSC.Codegen.JSNativeBrotli.getConstructor;

pub const SNativeBrotli = struct {
pub usingnamespace bun.NewRefCounted(@This(), deinit);
pub usingnamespace JSC.Codegen.JSNativeZlib;
pub usingnamespace JSC.Codegen.JSNativeBrotli;
pub usingnamespace CompressionStream(@This());

ref_count: u32 = 1,
mode: bun.zlib.NodeMode,
globalThis: *JSC.JSGlobalObject,
stream: BrotliContext = .{},
write_result: ?[*]u32 = null,
write_callback: JSC.Strong = .{},
onerror_value: JSC.Strong = .{},
poll_ref: CountedKeepAlive = .{},
this_value: JSC.Strong = .{},
write_in_progress: bool = false,
Expand Down Expand Up @@ -718,19 +725,22 @@ pub const SNativeBrotli = struct {

pub fn init(this: *@This(), globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
const arguments = callframe.argumentsUndef(3).slice();
const this_value = callframe.this();
if (arguments.len != 3) {
return globalThis.ERR_MISSING_ARGS("init(params, writeResult, writeCallback)", .{}).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]`.
const writeResult = arguments[1].asArrayBuffer(globalThis).?.asU32().ptr;
const writeCallback = try validators.validateFunction(globalThis, arguments[2], "writeCallback", .{});

this.write_result = writeResult;
this.write_callback.set(globalThis, writeCallback);

SNativeBrotli.writeCallbackSetCached(this_value, globalThis, writeCallback);

var err = this.stream.init();
if (err.isError()) {
try this.emitError(globalThis, err);
try this.emitError(globalThis, this_value, err);
return JSC.jsBoolean(false);
}

Expand Down Expand Up @@ -759,9 +769,12 @@ pub const SNativeBrotli = struct {
}

pub fn deinit(this: *@This()) void {
this.write_callback.deinit();
this.onerror_value.deinit();
this.this_value.deinit();
this.poll_ref.deinit();
switch (this.stream.mode) {
.BROTLI_ENCODE, .BROTLI_DECODE => this.stream.close(),
else => {},
}
this.destroy();
}
};
Expand Down
6 changes: 5 additions & 1 deletion src/codegen/class-definitions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ export interface ClassDefinition {
values?: string[];
JSType?: string;
noConstructor?: boolean;
wantsThis?: boolean;

// Do not try to track the `this` value in the constructor automatically.
// That is a memory leak.
wantsThis?: never;

/**
* Called from any thread.
*
Expand Down
24 changes: 0 additions & 24 deletions src/codegen/generate-classes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -361,12 +361,6 @@ JSC_DECLARE_CUSTOM_GETTER(js${typeName}Constructor);
`;
}

if (obj.wantsThis) {
externs += `
extern JSC_CALLCONV void* JSC_HOST_CALL_ATTRIBUTES ${classSymbolName(typeName, "_setThis")}(JSC::JSGlobalObject*, void*, JSC::EncodedJSValue);
`;
}

if (obj.structuredClone) {
externs +=
`extern JSC_CALLCONV void JSC_HOST_CALL_ATTRIBUTES ${symbolName(
Expand Down Expand Up @@ -652,13 +646,6 @@ JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${name}::construct(JSC::JSGlobalObj
}
auto value = JSValue::encode(instance);
${
obj.wantsThis
? `
${classSymbolName(typeName, "_setThis")}(globalObject, ptr, value);
`
: ""
}
RELEASE_AND_RETURN(scope, value);
}
Expand Down Expand Up @@ -1661,7 +1648,6 @@ function generateZig(
construct,
finalize,
noConstructor = false,
wantsThis = false,
overridesToJS = false,
estimatedSize,
call = false,
Expand Down Expand Up @@ -1794,16 +1780,6 @@ const JavaScriptCoreBindings = struct {
`;
}

if (construct && !noConstructor && wantsThis) {
exports.set("_setThis", classSymbolName(typeName, "_setThis"));
output += `
pub fn ${classSymbolName(typeName, "_setThis")}(globalObject: *JSC.JSGlobalObject, ptr: *anyopaque, this: JSC.JSValue) callconv(JSC.conv) void {
const real: *${typeName} = @ptrCast(@alignCast(ptr));
real.this_value.set(globalObject, this);
}
`;
}

if (call) {
exports.set("call", classSymbolName(typeName, "call"));
output += `
Expand Down
14 changes: 14 additions & 0 deletions test/js/node/test/common/gc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,22 @@ async function checkIfCollectableByCounting(fn, ctor, count, waitTime = 20) {
throw new Error(`${name} cannot be collected`);
}

var finalizationRegistry = new FinalizationRegistry(heldValue => {
heldValue.ongc();
})

function onGC(value, holder) {
if (holder?.ongc) {

finalizationRegistry.register(value, { ongc: holder.ongc });
}
}

module.exports = {
checkIfCollectable,
runAndBreathe,
checkIfCollectableByCounting,
onGC,
};


Loading

0 comments on commit 4f8a6b3

Please sign in to comment.