From 8be4fb61d0fbb3734a6b641af985da7dc350d2ec Mon Sep 17 00:00:00 2001 From: Zack Radisic <56137411+zackradisic@users.noreply.github.com> Date: Wed, 10 Sep 2025 21:33:40 -0700 Subject: [PATCH] make it better --- src/bake/DevServer.zig | 2 +- src/bun.js/api/server/AnyRequestContext.zig | 16 +++++++-------- src/bun.js/api/server/RequestContext.zig | 22 +++++++++++++++++---- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/bake/DevServer.zig b/src/bake/DevServer.zig index 77745de49b..1304a99608 100644 --- a/src/bake/DevServer.zig +++ b/src/bake/DevServer.zig @@ -1116,7 +1116,7 @@ fn deferRequest( }, .saved => |saved| saved, }; - server_handler.ctx.setAbortCallback(DeferredRequest.onAbortWrapper, &deferred.data); + server_handler.ctx.setAdditionalOnAbortCallback(.{ .cb = DeferredRequest.onAbortWrapper, .data = &deferred.data }); break :brk .{ .server_handler = server_handler, }; diff --git a/src/bun.js/api/server/AnyRequestContext.zig b/src/bun.js/api/server/AnyRequestContext.zig index 51c3448b5e..9b777a2e6f 100644 --- a/src/bun.js/api/server/AnyRequestContext.zig +++ b/src/bun.js/api/server/AnyRequestContext.zig @@ -18,27 +18,23 @@ pub fn init(request_ctx: anytype) AnyRequestContext { return .{ .tagged_pointer = Pointer.init(request_ctx) }; } -pub fn setAbortCallback(self: AnyRequestContext, cb: *const fn (this: *anyopaque) void, data: *anyopaque) void { +pub fn setAdditionalOnAbortCallback(self: AnyRequestContext, cb: ?AdditionalOnAbortCallback) void { if (self.tagged_pointer.isNull()) { return; } switch (self.tagged_pointer.tag()) { @field(Pointer.Tag, bun.meta.typeBaseName(@typeName(HTTPServer.RequestContext))) => { - self.tagged_pointer.as(HTTPServer.RequestContext).onAbortCb = cb; - self.tagged_pointer.as(HTTPServer.RequestContext).onAbortData = data; + self.tagged_pointer.as(HTTPServer.RequestContext).additional_on_abort = cb; }, @field(Pointer.Tag, bun.meta.typeBaseName(@typeName(HTTPSServer.RequestContext))) => { - self.tagged_pointer.as(HTTPSServer.RequestContext).onAbortCb = cb; - self.tagged_pointer.as(HTTPSServer.RequestContext).onAbortData = data; + self.tagged_pointer.as(HTTPSServer.RequestContext).additional_on_abort = cb; }, @field(Pointer.Tag, bun.meta.typeBaseName(@typeName(DebugHTTPServer.RequestContext))) => { - self.tagged_pointer.as(DebugHTTPServer.RequestContext).onAbortCb = cb; - self.tagged_pointer.as(DebugHTTPServer.RequestContext).onAbortData = data; + self.tagged_pointer.as(DebugHTTPServer.RequestContext).additional_on_abort = cb; }, @field(Pointer.Tag, bun.meta.typeBaseName(@typeName(DebugHTTPSServer.RequestContext))) => { - self.tagged_pointer.as(DebugHTTPSServer.RequestContext).onAbortCb = cb; - self.tagged_pointer.as(DebugHTTPSServer.RequestContext).onAbortData = data; + self.tagged_pointer.as(DebugHTTPSServer.RequestContext).additional_on_abort = cb; }, else => @panic("Unexpected AnyRequestContext tag"), } @@ -277,3 +273,5 @@ const DebugHTTPSServer = bun.api.DebugHTTPSServer; const DebugHTTPServer = bun.api.DebugHTTPServer; const HTTPSServer = bun.api.HTTPSServer; const HTTPServer = bun.api.HTTPServer; + +pub const AdditionalOnAbortCallback = @import("./RequestContext.zig").AdditionalOnAbortCallback; diff --git a/src/bun.js/api/server/RequestContext.zig b/src/bun.js/api/server/RequestContext.zig index 49b7edff43..1fb30d3e16 100644 --- a/src/bun.js/api/server/RequestContext.zig +++ b/src/bun.js/api/server/RequestContext.zig @@ -1,3 +1,18 @@ +/// Q: Why is this needed? +/// A: The dev server needs to attach its own callback when the request is +/// aborted. +/// +/// Q: Why can't the dev server just call `.setAbortHandler(...)` then? +/// A: It can't, because that is *already* called by the RequestContext, setting +/// the callback and the user data context pointer. +/// +/// If it did, it would *overwrite* the user data context pointer (this +/// is what it did before), causing segfaults. +pub const AdditionalOnAbortCallback = struct { + cb: *const fn (this: *anyopaque) void, + data: *anyopaque, +}; + pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comptime ThisServer: type) type { return struct { const RequestContext = @This(); @@ -55,8 +70,7 @@ pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, /// Defer finalization until after the request handler task is completed? defer_deinit_until_callback_completes: ?*bool = null, - onAbortCb: ?*const fn (this: *anyopaque) void = null, - onAbortData: ?*anyopaque = null, + additional_on_abort: ?AdditionalOnAbortCallback = null, // TODO: support builtin compression const can_sendfile = !ssl_enabled and !Environment.isWindows; @@ -584,8 +598,8 @@ pub fn NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, assert(this.server != null); // mark request as aborted this.flags.aborted = true; - if (this.onAbortData != null and this.onAbortCb != null) { - this.onAbortCb.?(this.onAbortData.?); + if (this.additional_on_abort) |abort| { + abort.cb(abort.data); } this.detachResponse();