From 83cf798a79da90282d4da2f947ecc71fb0d81aa9 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 2 Apr 2025 16:23:29 -0700 Subject: [PATCH] Remove JSC.Strong from NodeHTTPResponse --- src/bun.js/api/server.classes.ts | 2 +- src/bun.js/api/server.zig | 24 ++++++--------- src/bun.js/api/server/NodeHTTPResponse.zig | 36 +++++++++++----------- src/bun.js/bindings/NodeHTTP.cpp | 14 ++++++--- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/bun.js/api/server.classes.ts b/src/bun.js/api/server.classes.ts index 0584b7a23f..cd7c073355 100644 --- a/src/bun.js/api/server.classes.ts +++ b/src/bun.js/api/server.classes.ts @@ -191,7 +191,7 @@ export default [ klass: {}, finalize: true, noConstructor: true, - values: ["onAborted", "onWritable", "onData"], + values: ["onAborted", "onWritable", "onData", "promise"], }), define({ diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index dae3266d54..66bf4a9779 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -6429,6 +6429,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp const vm = this.vm; var node_http_response: ?*NodeHTTPResponse = null; + var node_response_jsvalue = JSC.JSValue.zero; var is_async = false; defer { if (!is_async) { @@ -6447,6 +6448,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp resp, upgrade_ctx, &node_http_response, + &node_response_jsvalue, ); const HTTPResult = union(enum) { @@ -6455,7 +6457,6 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp success: void, pending: JSC.JSValue, }; - var strong_promise: JSC.Strong = .empty; var needs_to_drain = true; defer { @@ -6463,7 +6464,6 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp vm.drainMicrotasks(); } } - defer strong_promise.deinit(); const http_result: HTTPResult = brk: { if (result.toError()) |err| { break :brk .{ .exception = err }; @@ -6471,7 +6471,9 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp if (result.asAnyPromise()) |promise| { if (promise.status(globalThis.vm()) == .pending) { - strong_promise.set(globalThis, result); + if (node_response_jsvalue != .zero) { + JSC.Codegen.JSNodeHTTPResponse.promiseSetCached(node_response_jsvalue, globalThis, result); + } needs_to_drain = false; vm.drainMicrotasks(); } @@ -6489,20 +6491,12 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp globalThis.handleRejectedPromises(); if (node_http_response) |node_response| { if (node_response.flags.request_has_completed or node_response.flags.socket_closed or node_response.flags.upgraded) { - strong_promise.deinit(); + if (node_response_jsvalue != .zero) + JSC.Codegen.JSNodeHTTPResponse.promiseSetCached(node_response_jsvalue, globalThis, .zero); break :brk .{ .success = {} }; } - const strong_self = node_response.getThisValue(); - - if (strong_self.isEmptyOrUndefinedOrNull()) { - strong_promise.deinit(); - break :brk .{ .success = {} }; - } - - node_response.promise = strong_promise; - strong_promise = .empty; - result._then2(globalThis, strong_self, NodeHTTPResponse.Bun__NodeHTTPRequest__onResolve, NodeHTTPResponse.Bun__NodeHTTPRequest__onReject); + result._then2(globalThis, node_response_jsvalue, NodeHTTPResponse.Bun__NodeHTTPRequest__onResolve, NodeHTTPResponse.Bun__NodeHTTPRequest__onReject); is_async = true; } @@ -7613,6 +7607,7 @@ extern fn NodeHTTPServer__onRequest_http( response: *uws.NewApp(false).Response, upgrade_ctx: ?*uws.uws_socket_context_t, node_response_ptr: *?*NodeHTTPResponse, + node_response_jsvalue: *JSC.JSValue, ) JSC.JSValue; extern fn NodeHTTPServer__onRequest_https( @@ -7624,6 +7619,7 @@ extern fn NodeHTTPServer__onRequest_https( response: *uws.NewApp(true).Response, upgrade_ctx: ?*uws.uws_socket_context_t, node_response_ptr: *?*NodeHTTPResponse, + node_response_jsvalue: *JSC.JSValue, ) JSC.JSValue; extern fn NodeHTTP_assignOnCloseFunction(bool, *anyopaque) void; diff --git a/src/bun.js/api/server/NodeHTTPResponse.zig b/src/bun.js/api/server/NodeHTTPResponse.zig index a9e095374e..b89b1cab9d 100644 --- a/src/bun.js/api/server/NodeHTTPResponse.zig +++ b/src/bun.js/api/server/NodeHTTPResponse.zig @@ -6,7 +6,6 @@ js_ref: JSC.Ref = .{}, flags: Flags = .{}, body_read_state: BodyReadState = .none, body_read_ref: JSC.Ref = .{}, -promise: JSC.Strong = .empty, server: AnyServer, /// When you call pause() on the node:http IncomingMessage @@ -598,16 +597,17 @@ pub fn onRequestComplete(this: *NodeHTTPResponse) void { pub export fn Bun__NodeHTTPRequest__onResolve(globalObject: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) callconv(JSC.conv) JSC.JSValue { log("onResolve", .{}); const arguments = callframe.arguments_old(2).slice(); - const this: *NodeHTTPResponse = arguments[1].as(NodeHTTPResponse).?; - this.promise.deinit(); + const this_jsvalue = arguments[1]; + const this: *NodeHTTPResponse = this_jsvalue.as(NodeHTTPResponse).?; + defer this_jsvalue.ensureStillAlive(); + + JSC.Codegen.JSNodeHTTPResponse.promiseSetCached(this_jsvalue, globalObject, .zero); + defer this.deref(); this.maybeStopReadingBody(globalObject.bunVM(), arguments[1]); if (!this.flags.request_has_completed and !this.flags.socket_closed) { - const this_value = this.getThisValue(); - if (this_value != .zero) { - NodeHTTPResponse.onAbortedSetCached(this_value, globalObject, .zero); - } + JSC.Codegen.JSNodeHTTPResponse.onAbortedSetCached(this_jsvalue, globalObject, .zero); this.raw_response.clearOnData(); this.raw_response.clearOnWritable(); this.raw_response.clearTimeout(); @@ -623,17 +623,17 @@ pub export fn Bun__NodeHTTPRequest__onResolve(globalObject: *JSC.JSGlobalObject, pub export fn Bun__NodeHTTPRequest__onReject(globalObject: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) callconv(JSC.conv) JSC.JSValue { const arguments = callframe.arguments_old(2).slice(); const err = arguments[0]; - const this: *NodeHTTPResponse = arguments[1].as(NodeHTTPResponse).?; - this.promise.deinit(); - this.maybeStopReadingBody(globalObject.bunVM(), arguments[1]); + const this_jsvalue = arguments[1]; + const this: *NodeHTTPResponse = this_jsvalue.as(NodeHTTPResponse).?; + defer this_jsvalue.ensureStillAlive(); + const vm = globalObject.bunVM(); + this.maybeStopReadingBody(vm, this_jsvalue); + JSC.Codegen.JSNodeHTTPResponse.promiseSetCached(this_jsvalue, globalObject, .zero); defer this.deref(); if (!this.flags.request_has_completed and !this.flags.socket_closed) { - const this_value = this.getThisValue(); - if (this_value != .zero) { - NodeHTTPResponse.onAbortedSetCached(this_value, globalObject, .zero); - } + JSC.Codegen.JSNodeHTTPResponse.onAbortedSetCached(this_jsvalue, globalObject, .zero); this.raw_response.clearOnData(); this.raw_response.clearOnWritable(); this.raw_response.clearTimeout(); @@ -644,7 +644,7 @@ pub export fn Bun__NodeHTTPRequest__onReject(globalObject: *JSC.JSGlobalObject, this.onRequestComplete(); } - _ = globalObject.bunVM().uncaughtException(globalObject, err, true); + _ = vm.uncaughtException(globalObject, err, true); return .undefined; } @@ -1063,10 +1063,10 @@ pub fn deinit(this: *NodeHTTPResponse) void { bun.debugAssert(this.flags.socket_closed or this.flags.request_has_completed); this.buffered_request_body_data_during_pause.deinitWithAllocator(bun.default_allocator); - this.js_ref.unref(JSC.VirtualMachine.get()); - this.body_read_ref.unref(JSC.VirtualMachine.get()); + const vm = JSC.VirtualMachine.get(); + this.js_ref.unref(vm); + this.body_read_ref.unref(vm); - this.promise.deinit(); this.destroy(); } diff --git a/src/bun.js/bindings/NodeHTTP.cpp b/src/bun.js/bindings/NodeHTTP.cpp index 73a87667af..f7586e8213 100644 --- a/src/bun.js/bindings/NodeHTTP.cpp +++ b/src/bun.js/bindings/NodeHTTP.cpp @@ -941,7 +941,8 @@ static EncodedJSValue NodeHTTPServer__onRequest( uWS::HttpRequest* request, uWS::HttpResponse* response, void* upgrade_ctx, - void** nodeHttpResponsePtr) + void** nodeHttpResponsePtr, + EncodedJSValue* nodeHttpResponseValue) { auto& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); @@ -960,6 +961,7 @@ static EncodedJSValue NodeHTTPServer__onRequest( bool hasBody = false; WebCore::JSNodeHTTPResponse* nodeHTTPResponseObject = jsCast(JSValue::decode(NodeHTTPResponse__createForJS(any_server, globalObject, &hasBody, request, isSSL, response, upgrade_ctx, nodeHttpResponsePtr))); + *nodeHttpResponseValue = JSValue::encode(nodeHTTPResponseObject); args.append(nodeHTTPResponseObject); args.append(jsBoolean(hasBody)); @@ -1188,9 +1190,10 @@ extern "C" EncodedJSValue NodeHTTPServer__onRequest_http( uWS::HttpRequest* request, uWS::HttpResponse* response, void* upgrade_ctx, - void** nodeHttpResponsePtr) + void** nodeHttpResponsePtr, + EncodedJSValue* nodeHttpResponseValue) { - return NodeHTTPServer__onRequest(any_server, globalObject, JSValue::decode(thisValue), JSValue::decode(callback), request, response, upgrade_ctx, nodeHttpResponsePtr); + return NodeHTTPServer__onRequest(any_server, globalObject, JSValue::decode(thisValue), JSValue::decode(callback), request, response, upgrade_ctx, nodeHttpResponsePtr, nodeHttpResponseValue); } extern "C" EncodedJSValue NodeHTTPServer__onRequest_https( @@ -1201,9 +1204,10 @@ extern "C" EncodedJSValue NodeHTTPServer__onRequest_https( uWS::HttpRequest* request, uWS::HttpResponse* response, void* upgrade_ctx, - void** nodeHttpResponsePtr) + void** nodeHttpResponsePtr, + EncodedJSValue* nodeHttpResponseValue) { - return NodeHTTPServer__onRequest(any_server, globalObject, JSValue::decode(thisValue), JSValue::decode(callback), request, response, upgrade_ctx, nodeHttpResponsePtr); + return NodeHTTPServer__onRequest(any_server, globalObject, JSValue::decode(thisValue), JSValue::decode(callback), request, response, upgrade_ctx, nodeHttpResponsePtr, nodeHttpResponseValue); } JSC_DEFINE_HOST_FUNCTION(jsHTTPAssignHeaders, (JSGlobalObject * globalObject, CallFrame* callFrame))