mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-11 03:18:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix fetch.zig to use bun.default_allocator directly instead of AllocationScope

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
Claude Bot
2025-10-31 12:40:36 +00:00
parent 6b525b086d
commit da0deb083c
1 changed files with 10 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
src/bun.js/webcore/fetch.zig
View File

@@ -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,