Skip to content

net: reserved socket handle ids leak (reserve_handle_id never freed); exhaustion panics #6441

Description

@proggeramlug

Follow-up to #6407, which fixed the ext-lib handle-id collision (sockets and http-server ids sharing the [1, 0x40000) band). The remaining half is the leak: reserve_handle_id() mints an id per socket and nothing ever returns it, and next_fresh_handle_id() panic!s on exhaustion — so a long-running server crashes after ~262k accepted connections.

Why it wasn't fixed in #6407

The naive fix (free the id at 'close') is unsafe: a net.Socket stays inspectable after 'close' in Node (socket.destroyed, a late write), so a stale JS reference would dispatch on a recycled id and alias a new socket — trading a slow leak for fast cross-object corruption, the exact aliasing class #6407 fixes. The socket id's lifetime is the JS object's, not the TCP connection's.

The architectural constraint

The socket value JS holds is a bare handle-band id (js_net_socket_alloc returns a raw i64), not a heap ObjectHeader. The GC skips handle-band ids, so there's no ordinary finalizer to hook. A correct free-when-unreachable needs one of:

  1. Socket as a GC heap object — represent net.Socket as an ObjectHeader holding the id in a field, and hang a GcFinalizeHookKind finalizer (like fix(gc): a typed array's backing ArrayBuffer was invisible to the collector #6408 did for typed-array backings) that calls free_handle_id(id) on collection. This is the clean fix but it rewrites ext-net's object model: ~36 socket method sites and the pervasive < 0x100000 guards in jsvalue.rs all assume the socket is a bare id, so it's a substantial, critical-path (mysql2/http/Next.js) change.
  2. GC handle-band liveness sweep — have the collector report which handle-band ids are reachable from roots and free reserved ids not in that set. Touches the hot GC path.

Starting point

The free_handle_id / free_handle_id_until primitive is the piece both approaches need: it recycles a reserve_handle_id id through the existing quarantine (recycle_handle / recycle_handle_until), so a stale bare reference dispatched before the next drain_quarantined_handles tick spends against an empty slot rather than aliasing. (Drafted during #6407 review; not yet committed — belongs in this fix's PR.)

Interim de-risk if the full fix is deferred: make next_fresh_handle_id degrade (JS-visible error) instead of panic!, so exhaustion is recoverable rather than a process crash.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions