Quarantine test-net-connect-memleak on linux-x64-musl

[robobun]

Fixes #33044

What

test/js/node/test/parallel/test-net-connect-memleak.js fails on roughly half of PR builds on the two musl test lanes (:alpine: 3.23 x64 - test-bun and :alpine: 3.23 x64-baseline - test-bun). The per-file retry also fails, so the lane goes red. The glibc linux x64 / x64-baseline / ASAN / darwin / Windows lanes do not hit it.

51 | function done(sock) {
52 |   globalThis.gc();
53 |   setImmediate(common.mustCall(() => {
54 |     assert.strictEqual(collected, true);
                ^
AssertionError: Expected values to be strictly equal:

false !== true

      at <anonymous> (/var/lib/buildkite-agent/build/test/js/node/test/parallel/test-net-connect-memleak.js:54:12)

Examples: build 66653, build 66657. These are unrelated PRs, so this is not caused by any one change.

Cause

The test registers an object in a FinalizationRegistry (via common/gc.js onGC), then asserts the cleanup callback has fired within one globalThis.gc() plus one setImmediate. The FinalizationRegistry spec gives no timing guarantee for cleanup callbacks; JSC schedules them through DeferredWorkTimer with no defined ordering relative to the immediate queue. On musl x64 that delivery intermittently slips past the single setImmediate, so collected is still false when the assertion runs. The object is collected and the listener is removed; only the delivery timing is racy.

This is the same mechanism already on record for the test's TLS sibling, test-tls-connect-memleak.js, which was quarantined on this exact matrix (test/expectations.txt). Both tests use the identical onGC + gc() + setImmediate pattern.

Fix

These are verbatim upstream Node ports that we do not edit (test/js/node/test/parallel/CLAUDE.md), so the robust in-test fix (gcUntil() instead of a single tick) is not available here. Mark the net twin [ FLAKY ] on [ LINUX-X64-MUSL ] next to its tls sibling and fold both under one comment. The LINUX-X64-MUSL modifier covers both the regular and baseline musl x64 lanes; the test keeps running everywhere else.

Verification

Parsing test/expectations.txt with the runner's own logic (scripts/runner.node.mjs getTestExpectations) and simulating each lane's modifiers:

• musl x64 lane modifiers -> both test-tls-connect-memleak.js and test-net-connect-memleak.js are quarantined.
• glibc x64 lane modifiers -> neither is quarantined; both still run.

The failure is musl-x64 only and does not reproduce on glibc, so there is no local fail-before to capture off that platform; the change is test-infra only (no source change).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 976cc98e-314c-4b58-b499-93bf04085c9c

📥 Commits

Reviewing files that changed from the base of the PR and between f789198 and 3e810cf.

📒 Files selected for processing (1)

• test/expectations.txt

---

Walkthrough

test/expectations.txt is updated to mark test-net-connect-memleak.js as [ FLAKY ] on LINUX-X64-MUSL, alongside the already-quarantined test-tls-connect-memleak.js. The accompanying comment explaining the FinalizationRegistry GC timing race is expanded.

Flaky test expectation update

Layer / File(s)	Summary
Add net memleak flaky entry and update commentary test/expectations.txt	Adds test-net-connect-memleak.js as [ FLAKY ] on LINUX-X64-MUSL next to its TLS sibling, and replaces the short comment with a longer explanation of the FinalizationRegistry callback vs. setImmediate timing race on musl x64.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly states the main change: quarantining test-net-connect-memleak on linux-x64-musl.
Description check	✅ Passed	The description covers what changed and how it was verified, matching the template's intent despite different headings.
Linked Issues check	✅ Passed	The PR matches #33044 by adding a flaky expectation for test-net-connect-memleak.js on LINUX-X64-MUSL alongside the existing TLS twin.
Out of Scope Changes check	✅ Passed	The only change is an expectations update in test/expectations.txt, which stays within the issue scope.

---

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

[robobun]

^{Updated 3:25 AM PT - Jun 29th, 2026}

❌ @robobun, your commit 3e810cf has 1 failures in Build #66690 (All Failures):

• test/js/web/websocket/autobahn.test.ts - code 1 on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 33045

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

bun-33045 --bun

[claude]

LGTM — test-infra only, adds the net twin next to its already-quarantined tls sibling on the same musl-x64 modifier.

Extended reasoning...

Overview

This PR touches a single file, test/expectations.txt, adding one [ FLAKY ] quarantine line for test/js/node/test/parallel/test-net-connect-memleak.js scoped to [ LINUX-X64-MUSL ], and rewording the existing comment block to cover both the tls and net variants together. The tls sibling was already quarantined on this exact modifier with the same rationale (FinalizationRegistry callback delivery vs setImmediate timing under JSC on musl x64). No source code or test code is changed.

Security risks

None. This is a CI test-expectations metadata file consumed only by scripts/runner.node.mjs to decide which test outcomes are tolerated on which lanes. It has no bearing on the shipped binary, auth, crypto, or any runtime path.

Level of scrutiny

Low. The change is mechanical and follows an established pattern already present one line above for the tls twin. I confirmed the LINUX-X64-MUSL modifier is produced by the runner's getTestModifiers (${os}-${arch}-${abi} uppercased), so the scoping is correct and matches the existing entry. The quarantined test file exists in the repo. The [ FLAKY ] expectation means the test still runs and only tolerates intermittent failure on the two Alpine x64 lanes; all other platforms continue to gate on it.

Other factors

The bug-hunting system found no issues. There are no outstanding human reviewer comments. The PR description provides build links demonstrating the flake on unrelated PRs and explains why an in-test fix isn't available (verbatim upstream Node port policy). This is well within the bounds of a routine flaky-test quarantine.

[claude]

No issues found — this mirrors the existing test-tls-connect-memleak.js quarantine line exactly (same [ LINUX-X64-MUSL ] modifier, same [ FLAKY ] expectation). Deferring since test/expectations.txt has a CODEOWNER.

Extended reasoning...

Overview

This PR adds a single [ FLAKY ] quarantine entry to test/expectations.txt for test/js/node/test/parallel/test-net-connect-memleak.js on the [ LINUX-X64-MUSL ] matrix, and rewords the comment block above it to cover both the tls and net memleak twins under one explanation. The new line is byte-for-byte the same shape as the pre-existing test-tls-connect-memleak.js entry directly above it. No source code is touched.

Security risks

None. This is a test-infrastructure-only change to a plain-text expectations file consumed by the CI runner. It does not affect the built binary, does not change runtime behavior, and does not touch auth/crypto/permissions.

Level of scrutiny

Low on the mechanics: the format, modifier, and expectation token all match an adjacent entry already accepted into the file, the target test file exists, and the rationale (FinalizationRegistry callback timing vs a single setImmediate on musl) is the same one already documented for the tls sibling. The only judgment call here is whether quarantining (vs. fixing) is the right policy, which is exactly what the CODEOWNER designation on this file exists to gate.

Other factors

.github/CODEOWNERS assigns /test/expectations.txt to a specific owner, so I'm deferring rather than approving. The bug-hunting pass found nothing; I verified the test path exists and that the new line follows the established pattern. The robobun CI comment shows unrelated failures on build #66669, which is expected for a quarantine-only change that doesn't fix other lanes.

[robobun]

Ready to merge: diff is green, only CI red is unrelated infra

This change adds a single [ FLAKY ] line to test/expectations.txt for the musl x64 lanes, and those lanes pass with it. In build #66669, :alpine: 3.23 x64 - test-bun and :alpine: 3.23 x64-baseline - test-bun were both 20/20 green, which is exactly the outcome this PR is meant to produce.

The CI red on both #66669 and the re-run #66690 is the same pair of :darwin: 26 aarch64 - test-bun jobs failing before any test runs:

Error: buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. Refusing to continue with a partial download (would silently fall back to the wrong binary).

That is a BuildKite artifact-download timeout on the darwin aarch64 lane, not a test failure, and it has no relationship to a musl-x64 expectations entry. It reproduced identically across two independent builds, so another re-run is unlikely to clear it.

Both automated reviews came back with no findings. test/expectations.txt has a CODEOWNER, so this is ready for the owner to merge.

[robobun]

The same failure is root-caused in #33225: it is conservative stack scanning keeping the removed listener's closure alive across the test's single gc(), not FinalizationRegistry delivery timing, and it now hits both alpine x64 lanes on every PR build. #33225 fixes the test itself (and the tls twin) and removes the existing tls quarantine instead of adding a second one. If its musl lanes come back green, this PR can be closed in favor of it.

[robobun]

Closing: #35182 (merged 2026-07-22, commit 7e06b7f) deleted test-net-connect-memleak.js and its tls twin from test/js/node/test/parallel/, and #33044 was closed on that basis. Neither file exists on current main, so there is nothing left to quarantine.