Networking: polish pass (local-port op, is_alive rename, IP-literal fast path, encapsulation refactor, single-flight DNS) #482

Closed
opened 2026-05-13 17:03:50 +00:00 by navicore · 1 comment
Owner

Networking surface — polish pass

Small tidy-ups deferred across the PR1–PR5 networking arc (#477, #478, #479, #480, #481). Each is independently mergeable; bundling them keeps the churn in one PR.

Items

  • net.tcp.local-port ( Socket -- Int Bool ) op (deferred from PR2 #478)
    No way today to read the OS-assigned port after net.tcp.listen 0. Integration tests currently hardcode ports as a workaround — tcp-test-port, tcp-test-port-2, tls-test-port-{,-bad,-dns}, tcp-test-port-bad. With local-port, tests can bind on 0 and pick up whatever the OS assigned, ending the magic-number-per-test pattern. Tiny.

  • pool::is_alive rename (deferred from PR4 #480)
    Reads as a property test, behaves as "safe to reuse." Rename to is_quiet or is_reusable. Adjust the module doc to match. Tiny.

  • IP-literal fast path in DNS resolver call sites (deferred from PR5 #481)
    dns::resolve("127.0.0.1") round-trips through the worker-pool channel + LRU cache even though getaddrinfo with the numeric-host arm is trivially fast. Adding a hostname.parse::<IpAddr>().ok() short-circuit at the three call sites — tcp::patch_seq_tcp_connect, udp::patch_seq_udp_send_to, http_client::ssrf::validate_url — is measurable on tight loops, irrelevant otherwise. Worth doing in one pass rather than per-PR.

    PR5 reviewer's framing:

    "A hostname.parse::<IpAddr>().ok() short-circuit before dns::resolve would skip the pool round-trip for literal calls — measurable on tight loops, irrelevant otherwise. Worth doing once across all three call sites rather than per-PR."

    Tiny.

  • Encapsulation refactor: move take_tcp + install-into-registry back into tcp.rs (deferred from PR3 #479)
    PR3 promoted STREAMS and SocketRegistry to pub(crate) so tls.rs and http_client/pool.rs could use them. With the surface settled, the take/install pattern can live inside tcp.rs and the registry types can return to fully private — re-localises the "free + allocate is dangerous across a yield" invariant inside the module that owns it.

    PR3 reviewer's framing:

    "The existing take_tcp helper in tls.rs is most of the way to a clean API; moving it (and its install_tls counterpart) into tcp.rs and keeping STREAMS/SocketRegistry private would re-localise the invariant that 'free + allocate is dangerous across a yield' inside the module that owns it."

    Small.

  • Single-flight DNS resolution (deferred from PR1 #477)
    N concurrent strands resolving the same uncached host enqueue N separate getaddrinfo jobs today — the cache only collapses sequential fanout. An in-flight map keyed by hostname (HashMap<String, Vec<reply_tx>>) collapses concurrent duplicates onto a single worker job. Cited in crates/runtime/src/dns.rs module doc and docs/STDLIB_REFERENCE.md net.dns section as a planned follow-up. Small.

Rough scope

Half-day's work. One PR. No design discussion needed; all items are surface-level changes against established APIs.

## Networking surface — polish pass Small tidy-ups deferred across the PR1–PR5 networking arc (#477, #478, #479, #480, #481). Each is independently mergeable; bundling them keeps the churn in one PR. ### Items - **`net.tcp.local-port ( Socket -- Int Bool )` op** (deferred from PR2 #478) No way today to read the OS-assigned port after `net.tcp.listen 0`. Integration tests currently hardcode ports as a workaround — `tcp-test-port`, `tcp-test-port-2`, `tls-test-port-{,-bad,-dns}`, `tcp-test-port-bad`. With `local-port`, tests can bind on `0` and pick up whatever the OS assigned, ending the magic-number-per-test pattern. **Tiny.** - **`pool::is_alive` rename** (deferred from PR4 #480) Reads as a property test, behaves as "safe to reuse." Rename to `is_quiet` or `is_reusable`. Adjust the module doc to match. **Tiny.** - **IP-literal fast path in DNS resolver call sites** (deferred from PR5 #481) `dns::resolve("127.0.0.1")` round-trips through the worker-pool channel + LRU cache even though `getaddrinfo` with the numeric-host arm is trivially fast. Adding a `hostname.parse::<IpAddr>().ok()` short-circuit at the three call sites — `tcp::patch_seq_tcp_connect`, `udp::patch_seq_udp_send_to`, `http_client::ssrf::validate_url` — is measurable on tight loops, irrelevant otherwise. Worth doing in one pass rather than per-PR. PR5 reviewer's framing: > "A `hostname.parse::<IpAddr>().ok()` short-circuit before `dns::resolve` would skip the pool round-trip for literal calls — measurable on tight loops, irrelevant otherwise. Worth doing once across all three call sites rather than per-PR." **Tiny.** - **Encapsulation refactor: move `take_tcp` + install-into-registry back into `tcp.rs`** (deferred from PR3 #479) PR3 promoted `STREAMS` and `SocketRegistry` to `pub(crate)` so `tls.rs` and `http_client/pool.rs` could use them. With the surface settled, the take/install pattern can live inside `tcp.rs` and the registry types can return to fully private — re-localises the "free + allocate is dangerous across a yield" invariant inside the module that owns it. PR3 reviewer's framing: > "The existing `take_tcp` helper in `tls.rs` is most of the way to a clean API; moving it (and its `install_tls` counterpart) into `tcp.rs` and keeping `STREAMS`/`SocketRegistry` private would re-localise the invariant that 'free + allocate is dangerous across a yield' inside the module that owns it." **Small.** - **Single-flight DNS resolution** (deferred from PR1 #477) N concurrent strands resolving the same uncached host enqueue N separate `getaddrinfo` jobs today — the cache only collapses *sequential* fanout. An in-flight map keyed by hostname (`HashMap<String, Vec<reply_tx>>`) collapses concurrent duplicates onto a single worker job. Cited in `crates/runtime/src/dns.rs` module doc and `docs/STDLIB_REFERENCE.md` net.dns section as a planned follow-up. **Small.** ### Rough scope Half-day's work. One PR. No design discussion needed; all items are surface-level changes against established APIs.
Author
Owner
https://git.navicore.tech/navicore/patch-seq/pulls/486
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
navicore/patch-seq#482
No description provided.