Skip to content

test(proxy-stress): bind deadPort()'s holder before it connects - #41336

Open
robobun wants to merge 2 commits into
mainfrom
robobun/e699489d/deflake-proxy-dead-port
Open

robobun wants to merge 2 commits into
mainfrom
robobun/e699489d/deflake-proxy-dead-port

Conversation

@robobun

@robobun robobun commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • proxy-stress-errors.test.ts flakes on Linux. The Alpine 3.23 x64 lane failed upstream unreachable via proxy > https-proxy, CONNECT upstream refused → 502 with TypeError: Unable to connect. Is the computer able to access the url? (code: "ConnectionRefused"). test(proxy-stress): return deadPort()'s bind(0) port, not the connect-autobound one #35400 saw Malformed_HTTP_Response from the same cause.
  • deadPort() (test/js/bun/http/proxy-stress-helpers.ts:721) returns a port that connect() picked. Linux can give that port as the source port to a later connect() to the same port. That socket connects to itself. The proxy then tunnels to itself and does not send the 502.

Fix

Background

  • A dead port is a port where nothing listens. A connect() to it fails with ECONNREFUSED.
  • A TCP self-connect occurs when the source and the destination (address and port) are equal. The connect succeeds, and each byte the socket writes comes back to it.
  • Linux picks the source port of a connect() in __inet_hash_connect. It can reuse a port that another connect() picked, if the 4-tuple is unique. It skips a port that bind() picked.
Notes

Probes: Linux 6.17 (port range 32768 to 60999) and Windows Server 2019, with bun canary.

deadPort() shape Linux: self-connects in 150000 dials Linux: listen(0) collisions (500 held, 20000 listens) Windows: listen(0) collisions (500 held, 20000 listens)
main: holder port, picked by connect() 11 0 0
#35400: listener port 0 0 500
this PR: holder port, picked by bind() 0 0 0
  • A self-connected upstream echoes each byte. An echo server in its place gives the CI errors: ConnectionRefused for an https target (the inner TLS handshake reads its own ClientHello, src/http/ProxyTunnel.rs:432), and Malformed_HTTP_Response for an http target.
  • 11 in 150000 is about 1 in 14000. That is the chance that a random even port in the range is the dead port. The file dials a dead port 6 times per run.
  • The Linux listen(0) column for main shows little. listen(0) prefers odd ports, and connect() prefers even ports.
  • Windows listen(0) allocates ports in sequence. So the test(proxy-stress): return deadPort()'s bind(0) port, not the connect-autobound one #35400 collision needs a wrap of the 16384-port range while a test holds the port. That is unlikely, but the port is not reserved.
  • On Linux and Windows, the sink accepted the holder before sink.close() in 500 of 500 runs. So the helper does not need to wait for the accept.
  • The holder's TIME_WAIT now falls on a port from the bind(0) range: one port per dead port, for 60 seconds.
  • Suites run with bun test test/js/bun/http/proxy-stress-*.test.ts: Linux release 852 pass and 1 skip, Linux ASAN debug build 853 pass, Windows x64 852 pass and 1 skip. proxy-stress-errors.test.ts alone: Linux release 30 of 30, ASAN 3 of 3, Windows 10 of 10.

no test proof · iteration 1 · no src or test change; test-proof not applicable

deadPort() returns the local port of a connected socket, and connect()
picked that port. On Linux, a later connect() to that port can get the
same port as its source port. The socket then connects to itself. The
proxy in proxy-stress-errors.test.ts then tunnels to itself, and the
test sees ConnectionRefused or Malformed_HTTP_Response, not a 502.

The holder now binds 127.0.0.1 (localAddress) before it connects.
Linux does not give a port that bind() picked to connect(), so the
self-connect cannot occur. The port stays bound, so listen(0) still
skips it.

Probe, 150000 dials to a dead port on Linux 6.17:
  before: 11 self-connects
  after:  0 self-connects

The helper keeps the holder's port. It does not return the listener's
port, as #35400 does. On Windows, listen(0) hands out the port of a
closed listener while its accepted socket is still open: 500 of 20000
listens in a probe, and 0 with this change.
@robobun

robobun commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator Author

Reproduced on Linux with the bun canary, without CI:

  • Mechanism: 150000 dials to a deadPort() port. 11 of them connected to themselves (local port equal to remote port). With this change: 0.
  • Symptom: an echo server as the proxy upstream (a self-connected socket echoes each byte) gives ConnectionRefused for the https target and Malformed_HTTP_Response for the http target. These are the two CI errors.

The change is in a test helper only. No runtime code changes.

CI: build 110054 passed 181 of 182 jobs, on every platform. No proxy-stress-* file failed in it or in build 110045. The red jobs are in tests that this change does not touch:

  • test/js/node/quic/quic-endpoint.test.ts: a LeakSanitizer leak in lsquic (Debian 13 x64 ASAN, build 110045).
  • test/napi/node-napi-tests/test/js-native-api/test_object/do.test.ts: a worker crash (Ubuntu 25.04 x64, build 110045).
  • test/cli/install/isolated-relink.test.ts: a hook timeout (Debian 13 x64 ASAN, build 110054).

The diff is ready for review.

@github-actions github-actions Bot added the claude label Sep 4, 2026
@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: b1d09519-6ca1-4452-a1f8-40c7988b3443

📥 Commits

Reviewing files that changed from the base of the PR and between 2c2b984 and c682fae.

📒 Files selected for processing (1)
  • test/js/bun/http/proxy-stress-helpers.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


Walkthrough

Changes

deadPort connection handling

Layer / File(s) Summary
Loopback binding and port rationale
test/js/bun/http/proxy-stress-helpers.ts
deadPort documents its port allocation rationale and binds the holder connection to 127.0.0.1 to avoid Linux source-port reuse and self-connections.

Merge Risk: ⚪ Minimal · up to c682f

The helper now binds its holder socket to loopback to prevent self-connections while retaining the reserved-port behavior. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the test helper and the primary fix: binding the holder socket before it connects. It is concise and directly related to the changes.
Description check ✅ Passed The description explains the Linux flake, the root cause, the fix, platform-specific behavior, and detailed verification results. It does not use the template headings exactly, but it provides all req…

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:28 PM PT - Sep 3rd, 2026

❌ @robobun, your commit 5f9a67e has 1 failures in Build #110054 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 41336

That installs a local version of the PR into your bun-41336 executable, so you can run:

bun-41336 --bun

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants