socket: set Handlers.mode=.client for Windows named-pipe Bun.connect - #30150
Merged
Jarred-Sumner merged 2 commits intoMay 4, 2026
Claude / Claude Code Review
completed
May 3, 2026 in 22m 12s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟣 Pre-existing | src/bun.js/api/bun/socket/Listener.zig:660-662 |
Pre-existing: connection.unix path leaks on Windows named-pipe non-TLS Bun.connect |
Annotations
Check notice on line 662 in src/bun.js/api/bun/socket/Listener.zig
claude / Claude Code Review
Pre-existing: connection.unix path leaks on Windows named-pipe non-TLS Bun.connect
Pre-existing (not introduced here), but since this PR is hardening lifecycle in this exact `if (isNamedPipe)` block: the **non-TLS** arm just below (~lines 731–775) never adopts `connection` — both the `prev_maybe_tcp` reuse path (`bun.assert(prev.connection == null)`, no assignment) and `TCPSocket.new(.{ .connection = null, ... })` drop the heap-owned `connection.unix` slice on the floor, and the branch returns `promise_value` so the `errdefer connection.deinit()` never fires. The TLS arm and t
Loading