fix(ffi,net): recycle reserved socket handle ids; degrade exhaustion instead of panicking (#6441) - #6600
Merged
proggeramlug merged 1 commit intoJul 18, 2026
Conversation
…instead of panicking (PerryTS#6441) `reserve_handle_id()` mints a globally-unique id per ext-net socket (follow-up to PerryTS#6407's collision fix), but nothing ever returned it and `next_fresh_handle_id()` `panic!`d on exhaustion — so a long-running server leaked the whole `[1, 0x40000)` band and crashed after ~262k accepted connections. perry-ffi: - Add `free_handle_id` / `free_handle_id_until`: recycle a `reserve_handle_id` id through the existing quarantine (one-tick and deadline-gated tiers), so a stale bare reference dispatched before the next `drain_quarantined_handles` tick spends against an empty slot instead of aliasing a freshly reserved id (the ABA class PerryTS#6407 fixes). This is the primitive both candidate free-when-unreachable fixes build on; it does no reachability analysis itself (a `net.Socket` id outlives its `'close'`), so wiring the actual free is deferred. - `next_fresh_handle_id` now returns `Option`; `reserve_handle_id` degrades to `INVALID_HANDLE` on exhaustion instead of aborting the process. `register_handle` still panics (it has no valid key to insert under, and its ids recycle via `drop_handle` so it does not leak). perry-ext-net: - Synchronous entry points (`new net.Socket()`, `createServer`, `net.connect`, `new net.BlockList()`, `new net.SocketAddress()`) route through `next_id_or_throw`, which throws a recoverable `EMFILE`-coded error on exhaustion instead of registering a phantom id-0 socket. - Background paths (accept loop, `adopt_upgraded_tcp_stream`) can't unwind to a JS frame, so they drop the connection rather than register under the `0` sentinel; the raw-upgrade caller aborts the upgrade and reclaims the incoming-message handle. - Extract the id helpers into `handle_ids.rs` to keep `lib.rs` under the file-size gate. Tests: 5 new perry-ffi unit tests pin reserved-id recycling, the ABA quarantine guarantee, the deadline-gated tier, the invalid-handle no-op, and the exhaustion boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughHandle allocation now distinguishes recoverable exhaustion from throwing allocation, recycles reserved ids through quarantine, and prevents invalid socket or upgrade dispatch state when allocation fails. ChangesHandle exhaustion and socket allocation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Fixes #6441. Follow-up to #6407, which fixed the ext-lib handle-id collision; this addresses the remaining leak + exhaustion panic.
Problem
reserve_handle_id()mints a globally-unique id per ext-net socket, but nothing ever returns it, andnext_fresh_handle_id()panic!d on exhaustion — so a long-running server leaks the whole[1, 0x40000)band and crashes after ~262k accepted connections.The full free-when-unreachable fix is a large, critical-path change (represent
net.Socketas a GC heap object, or add a GC handle-band liveness sweep — see the issue). Anet.Socketstays inspectable after'close', so its id's lifetime is the JS object's, not the TCP connection's, and freeing on'close'alone would just trade the leak for the fast cross-object aliasing #6407 fixes. That rewrite is deferred. This PR lands the two pieces the issue calls out as belonging here now: the shared primitive both candidate fixes need, and the exhaustion de-risk.Changes
perry-ffi (
handle.rs)free_handle_id/free_handle_id_until— recycle areserve_handle_idid through the existing quarantine (one-tick and deadline-gated tiers), mirroringdrop_handle/drop_handle_until. A stale bare reference dispatched before the nextdrain_quarantined_handlestick spends against an empty slot instead of aliasing a freshly reserved id. This is the building block the eventual free-when-unreachable fix wires up; it performs no reachability analysis itself (deliberately), so it is exported but not yet called from a free site.next_fresh_handle_idnow returnsOption;reserve_handle_iddegrades toINVALID_HANDLEon exhaustion instead of aborting.register_handlestill panics — it has no valid key to insert under, and its ids recycle viadrop_handle, so it isn't the leaker.perry-ext-net
new net.Socket(),createServer,net.connect,new net.BlockList(),new net.SocketAddress()) route throughnext_id_or_throw, which throws a recoverable, JS-visibleEMFILE-coded error on exhaustion (Node's behavior for fd exhaustion) rather than registering a phantom id-0 socket.adopt_upgraded_tcp_stream) have no JS frame to unwind to, so they drop the connection instead of registering under the0sentinel; the raw-upgrade caller in perry-ext-http-server aborts the upgrade and reclaims the incoming-message handle.handle_ids.rsto keeplib.rsunder the file-size gate.Tests
5 new perry-ffi unit tests: reserved-id recycling decouples fresh-id consumption from cumulative reservations (churn past the band size); the ABA quarantine guarantee for reserved ids; the deadline-gated tier; the
INVALID_HANDLEno-op; and the exhaustion boundary (unit-tested via an extracted pure helper, so it doesn't advance the process-wide counter past the band and break other tests).cargo test -p perry-ffi(17 pass) andcargo test -p perry-ext-net(23 pass) green locally; all three touched cratescargo checkclean.Notes for reviewers
free_handle_idprimitive is intentionally unwired — the actual free-on-unreachable is the deferred follow-up. Landing the primitive now (drafted during fix(net): dispatch net.connect/createConnection reached as a bound value, and stop ext-lib handle ids colliding #6407 review) is what the issue asks for.lintmay be red on an unrelated pre-existing main-side file-size failure (crates/perry-runtime/src/bigint.rs, 2201 lines); not introduced here.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Reliability