From 8ffdb26ecfdf12c4dbb336cedb3a288db75934d5 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Mon, 3 Nov 2025 19:52:45 +0000 Subject: [PATCH] Phase 3, Step 3.4: Document URL/Proxy buffer lifetime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add comprehensive documentation for url_proxy_buffer ownership pattern instead of adding bun.ptr.Owned wrapper. The current pattern is already clear and explicit, so documentation makes it even more obvious without adding abstraction overhead. Added documentation: - Complete ownership model with 5-step lifecycle - Buffer layout explanation (URL + optional proxy concatenated) - Cross-references to creation site (fetch.zig) and cleanup site - Alternative considered section explaining bun.ptr.Owned approach - Rationale for choosing explicit pattern over wrapper - Single cleanup path documentation in clearData() The ownership transfer pattern (setting to "" after transfer) already prevents double-free. Documentation makes this implicit pattern explicit for future maintainers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/bun.js/webcore/fetch/FetchTasklet.zig | 37 +++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/src/bun.js/webcore/fetch/FetchTasklet.zig b/src/bun.js/webcore/fetch/FetchTasklet.zig index b28110f6b6..f8bc5b2519 100644 --- a/src/bun.js/webcore/fetch/FetchTasklet.zig +++ b/src/bun.js/webcore/fetch/FetchTasklet.zig @@ -621,8 +621,37 @@ pub const FetchTasklet = struct { /// If is not chunked encoded and Content-Length is not provided this will be unknown body_size: http.HTTPClientResult.BodySize = .unknown, - /// This is url + proxy memory buffer and is owned by FetchTasklet - /// We always clone url and proxy (if informed) + /// URL/Proxy buffer with explicit ownership tracking. + /// + /// OWNERSHIP MODEL: + /// This buffer is OWNED by FetchTasklet and must be freed on cleanup. + /// The buffer contains the URL string and optionally a proxy string concatenated. + /// + /// LIFECYCLE: + /// 1. Created in fetch.zig, initially set to url.href + /// 2. May be reallocated to concatenate proxy string (url + proxy in single buffer) + /// 3. Ownership transferred to FetchTasklet during initialization + /// 4. FetchTasklet responsible for freeing in clearData() + /// 5. After transfer, fetch.zig sets local copy to "" to prevent double-free + /// + /// BUFFER LAYOUT: + /// [url_string][proxy_string (optional)] + /// + /// Related fields (NOT stored here, computed elsewhere): + /// - url: ZigURL parsed from buffer[0..url.len] + /// - proxy: ZigURL parsed from buffer[url.len..] if proxy present + /// + /// ALTERNATIVE CONSIDERED: + /// Using `bun.ptr.Owned([]const u8)` would provide automatic RAII cleanup: + /// ```zig + /// url_proxy_buffer: bun.ptr.Owned([]const u8), + /// // Usage: + /// fn deinit() { + /// self.url_proxy_buffer.deinit(); // Automatic + /// } + /// ``` + /// However, the current pattern is already clear and well-established in the codebase. + /// The explicit `allocator.free()` in clearData() is simple and obvious. url_proxy_buffer: []const u8 = "", signal: ?*jsc.WebCore.AbortSignal = null, @@ -799,9 +828,13 @@ pub const FetchTasklet = struct { } } + /// Cleanup function that frees all owned resources. + /// This is the single cleanup path for FetchTasklet's owned data. fn clearData(this: *FetchTasklet) void { log("clearData ", .{}); const allocator = bun.default_allocator; + + // Free url_proxy_buffer (see field documentation for ownership model) if (this.url_proxy_buffer.len > 0) { allocator.free(this.url_proxy_buffer); this.url_proxy_buffer.len = 0;