From da0deb083c3d3fb9dc487be5fd20e81cb8d1bf8a Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Fri, 31 Oct 2025 12:40:36 +0000 Subject: [PATCH] Fix fetch.zig to use bun.default_allocator directly instead of AllocationScope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit introduced AllocationScope but this caused issues: 1. AllocationScope was being copied by value from fetch() -> FetchOptions -> FetchTasklet 2. Each copy contained a pointer to the same internal State, causing double-free 3. AllocationScope is designed for temporary allocations that all get freed together, but fetch has allocations (response body data) that outlive the FetchTasklet The solution is to use bun.default_allocator directly throughout fetch.zig. This is equivalent to what MemoryReportingAllocator was doing (it just wrapped bun.default_allocator for memory accounting purposes). Changes: - Removed allocation_scope field from FetchTasklet struct - Removed allocation_scope field from FetchOptions struct - Removed AllocationScope initialization in fetch() function - Changed all allocator references to use bun.default_allocator directly - Removed all leakSlice() calls (no longer needed) - Removed allocation_scope.deinit() calls Tests pass: bun bd test test/js/bun/http/fetch-file-upload.test.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/bun.js/webcore/fetch.zig | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/src/bun.js/webcore/fetch.zig b/src/bun.js/webcore/fetch.zig index 8f8344b773..3150f4e431 100644 --- a/src/bun.js/webcore/fetch.zig +++ b/src/bun.js/webcore/fetch.zig @@ -86,7 +86,6 @@ pub const FetchTasklet = struct { promise: jsc.JSPromise.Strong, concurrent_task: jsc.ConcurrentTask = .{}, poll_ref: Async.KeepAlive = .{}, - allocation_scope: bun.AllocationScope, /// For Http Client requests /// when Content-Length is provided this represents the whole size of the request /// If chunked encoded this will represent the total received size (ignoring the chunk headers) @@ -255,7 +254,7 @@ pub const FetchTasklet = struct { fn clearData(this: *FetchTasklet) void { log("clearData ", .{}); - const allocator = this.allocation_scope.allocator(); + const allocator = bun.default_allocator; if (this.url_proxy_buffer.len > 0) { allocator.free(this.url_proxy_buffer); this.url_proxy_buffer.len = 0; @@ -314,15 +313,13 @@ pub const FetchTasklet = struct { this.clearData(); - var scope = this.allocation_scope; - const allocator = scope.allocator(); + const allocator = bun.default_allocator; if (this.http) |http_| { this.http = null; allocator.destroy(http_); } allocator.destroy(this); - scope.deinit(); } fn getCurrentResponse(this: *FetchTasklet) ?*Response { @@ -477,7 +474,6 @@ pub const FetchTasklet = struct { buffer_reset = false; if (!this.result.has_more) { var scheduled_response_buffer = this.scheduled_response_buffer.list; - this.allocation_scope.leakSlice(scheduled_response_buffer.allocatedSlice()); const body = response.getBodyValue(); // done resolve body var old = body.*; @@ -490,7 +486,7 @@ pub const FetchTasklet = struct { log("onBodyReceived body_value length={}", .{body_value.InternalBlob.bytes.items.len}); this.scheduled_response_buffer = .{ - .allocator = this.allocation_scope.allocator(), + .allocator = bun.default_allocator, .list = .{ .items = &.{}, .capacity = 0, @@ -884,9 +880,8 @@ pub const FetchTasklet = struct { var scheduled_response_buffer = this.scheduled_response_buffer.list; // This means we have received part of the body but not the whole thing if (scheduled_response_buffer.items.len > 0) { - this.allocation_scope.leakSlice(scheduled_response_buffer.allocatedSlice()); this.scheduled_response_buffer = .{ - .allocator = this.allocation_scope.allocator(), + .allocator = bun.default_allocator, .list = .{ .items = &.{}, .capacity = 0, @@ -932,14 +927,13 @@ pub const FetchTasklet = struct { } var scheduled_response_buffer = this.scheduled_response_buffer.list; - this.allocation_scope.leakSlice(scheduled_response_buffer.allocatedSlice()); const response = Body.Value{ .InternalBlob = .{ .bytes = scheduled_response_buffer.toManaged(bun.default_allocator), }, }; this.scheduled_response_buffer = .{ - .allocator = this.allocation_scope.allocator(), + .allocator = bun.default_allocator, .list = .{ .items = &.{}, .capacity = 0, @@ -1047,14 +1041,14 @@ pub const FetchTasklet = struct { fetch_tasklet.* = .{ .mutex = .{}, .scheduled_response_buffer = .{ - .allocator = fetch_options.allocation_scope.allocator(), + .allocator = bun.default_allocator, .list = .{ .items = &.{}, .capacity = 0, }, }, .response_buffer = MutableString{ - .allocator = fetch_options.allocation_scope.allocator(), + .allocator = bun.default_allocator, .list = .{ .items = &.{}, .capacity = 0, @@ -1070,7 +1064,6 @@ pub const FetchTasklet = struct { .signal = fetch_options.signal, .hostname = fetch_options.hostname, .tracker = jsc.Debugger.AsyncTaskTracker.init(jsc_vm), - .allocation_scope = fetch_options.allocation_scope, .check_server_identity = fetch_options.check_server_identity, .reject_unauthorized = fetch_options.reject_unauthorized, .upgraded_connection = fetch_options.upgraded_connection, @@ -1101,7 +1094,7 @@ pub const FetchTasklet = struct { // This task gets queued on the HTTP thread. fetch_tasklet.http.?.* = http.AsyncHTTP.init( - fetch_options.allocation_scope.allocator(), + bun.default_allocator, fetch_options.method, fetch_options.url, fetch_options.headers.entries, @@ -1294,7 +1287,6 @@ pub const FetchTasklet = struct { globalThis: ?*JSGlobalObject, // Custom Hostname hostname: ?[]u8 = null, - allocation_scope: bun.AllocationScope, check_server_identity: jsc.Strong.Optional = .empty, unix_socket_path: ZigString.Slice, ssl_config: ?*SSLConfig = null, @@ -1374,7 +1366,7 @@ pub const FetchTasklet = struct { if (task.scheduled_response_buffer.list.capacity > 0) { task.scheduled_response_buffer.deinit(); task.scheduled_response_buffer = .{ - .allocator = task.allocation_scope.allocator(), + .allocator = bun.default_allocator, .list = .{ .items = &.{}, .capacity = 0, @@ -1515,15 +1507,10 @@ pub fn Bun__fetch_( bun.analytics.Features.fetch += 1; const vm = jsc.VirtualMachine.get(); - var allocation_scope = bun.AllocationScope.init(bun.default_allocator); // used to clean up dynamically allocated memory on error (a poor man's errdefer) var is_error = false; var upgraded_connection = false; - var allocator = allocation_scope.allocator(); - errdefer allocation_scope.deinit(); - defer { - if (is_error) allocation_scope.deinit(); - } + var allocator = bun.default_allocator; if (arguments.len == 0) { const err = ctx.toTypeError(.MISSING_ARGS, fetch_error_no_args, .{}); @@ -2693,7 +2680,6 @@ pub fn Bun__fetch_( .globalThis = globalThis, .ssl_config = ssl_config, .hostname = hostname, - .allocation_scope = allocation_scope, .upgraded_connection = upgraded_connection, .check_server_identity = if (check_server_identity.isEmptyOrUndefinedOrNull()) .empty else .create(check_server_identity, globalThis), .unix_socket_path = unix_socket_path,