Skip to content

fix(s3): avoid double-freeing path when presign throws after store creation - #30495

Closed
robobun wants to merge 4 commits into
mainfrom
farm/cf450992/fix-s3-path-double-free
Closed

fix(s3): avoid double-freeing path when presign throws after store creation#30495
robobun wants to merge 4 commits into
mainfrom
farm/cf450992/fix-s3-path-double-free

fix(s3): avoid double-freeing path when presign/unlink/etc throw afte…

6439d14
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 11, 2026 in 16m 38s

Code review found 1 potential issue

Found 4 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/runtime/webcore/S3Client.zig:157-158 Double-free still possible when constructor throws after initS3

Annotations

Check warning on line 158 in src/runtime/webcore/S3Client.zig

See this annotation in the file changed.

@claude claude / Claude Code Review

Double-free still possible when constructor throws after initS3

The PR description says "clear the caller's path right after the store takes ownership", but the clear actually happens after the *constructor* returns — and the constructor calls `initS3` (which takes ownership) and then does `errdefer store.deinit()` followed by fallible JS calls like `try opts.getTruthyComptime(globalObject, "type")`. If that getter throws, `store.deinit()` frees the path, then the caller's `errdefer path.deinit()` fires on a path that was never cleared (the clear on line 158