Repository navigation
fix(net): schedule close events independently of user timers - #40
Merged
Merged
Conversation
steipete
force-pushed
the
codex/socket-close-internal-immediate
branch
from
September 30, 2026 10:11
a4b5965 to
d2d3657
Compare
steipete
force-pushed
the
codex/socket-close-internal-immediate
branch
from
September 30, 2026 10:24
d2d3657 to
25a1582
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Socket close notifications now use Bun's native immediate scheduler even when user code or a fake clock replaces global
setImmediateornode:timers.setImmediate.closeSocketHandlepreviously closed the native handle and then looked up ambientsetImmediateto emitclose. A replacement that never runs its callback strands the event after the peer has already sent FIN. Bind node:net's existing immediate continuations directly to the native timer implementation. Keeping the check-phase scheduler, rather than moving close into nextTick, preserves the ordering that Node exposes relative to end, finish, nextTick and real immediates. The same internal binding covers reset and adopted-TLS close continuations.Validation: four subprocess variants poison timers before/after importing net and cover peer/local initiation. All pass patched (317 ms including the test runner); both test groups fail at the explicit deadline on baseline e8105cb. Identical standalone probes pass on Node 24.21.0 and miss close in all four baseline cases. Net: 268 passed / 37 skipped. TLS: 457 passed / 32 skipped / 6 TODO. HTTP: 1,662 passed / 29 skipped / 3 TODO / 1 pre-existing Node-reference failure documented in #38. Source TypeScript, lint, formatting and independent P2 autoreview pass. No Rust changes. Rebased onto the separately merged TLS fix at a0ef910: only additive changelog entries conflicted; runtime/test/docs patch-id matches the tested original. Rebase review is clean; GitHub Lint and Format pass on d2d3657.
W15 measured 119.176 excess test-seconds in the old admin handler file and modeled 20–44 seconds of whole-config invocation opportunity. Those scheduling estimates are not measured patched speedups. Current OpenClaw main 95ed4ee478cd8, full admin handler file: baseline 12 passed / 1 timeout in 72.403 s; patched 13 passed in 46.040 s. Runs use one worker, private 0700 TMPDIR, the selected Bun first on PATH, OPENCLAW_VITEST_RUNTIME=bun, disabled disposable compile caching, 15 s test/hook deadlines and a 300 s outer guard. The 408 response and real client close both complete without advancing fake time further. Shared-host measurements are not isolated throughput benchmarks.
Upstream status: searches of oven-sh/bun issues and PRs for socket close/setImmediate and fake timers found no matching fix to port.