feat(net)!: inject an explicit Runtime for session timers and machines - #3007
feat(net)!: inject an explicit Runtime for session timers and machines#3007kixelated wants to merge 2 commits into
Conversation
moq-net never spawns onto an ambient executor and never reads ambient time again. Client::connect and Server::accept take a moq_net::Runtime (associated Transport and Timer types, a spawn for the session's protocol machine, and the clock its deadlines arm against) and return the plain Session; the public Driver type is gone, replaced by runtime::Machine handed straight to the runtime. Session-side timers (bandwidth sampling, GOAWAY enforcement, control and advertise timeouts, subscription linger, PROBE cadence) all arm through the injected runtime, so a thread-per-core io_uring runtime can drive sessions without tokio. The trait carries no Send bounds anywhere: pinning one transport type per runtime lets each implementation know the concrete machine it spawns, so moq-tokio keeps plain tokio::spawn (its transports are Send) while a pinned !Send transport stays expressible. moq-tokio ships runtime::Runtime<S> plus runtime::Inline (hands the machine back for callers that drive it in place, which the relay's WebSocket handler does); moq-wasm and moq-ffi ship the browser runtime; moq_net::runtime::Test (feature test-runtime) is a deterministic virtual-clock runtime for tests. Deviations from the design posted on #2875, discovered during implementation: Runtime keeps a defaulted now() because relative arming (now + interval) and stamp-derived deadlines must read the virtual clock in tests or an advance leaves them in the past; and the model layer also owns deadlines (origin serve/linger), so kio::time survives for model internals and its removal moves to the follow-up PR that migrates the model and origin driver. Part of #2875 (the runtime prerequisite for moq-uring). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCxzHiGxN5qi8Gd687nsgm
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5a8304969
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| R: crate::runtime::Runtime + MaybeSend + MaybeSync + 'static, | ||
| R::Timer: MaybeSend, |
There was a problem hiding this comment.
On non-wasm targets, MaybeSend/MaybeSync impose Send/Sync, so these bounds, together with R::Timer: MaybeSend and the native MaybeSendBox = dyn Future + Send, reject the thread-pinned !Send runtimes and machines this new API is intended to enable. A Linux io_uring runtime using local state cannot call connect/accept or box its protocol future, despite Runtime itself advertising no Send requirement. Make sendness follow the chosen runtime/transport rather than the compilation target before publishing this breaking API shape.
AGENTS.md reference: AGENTS.md:L150-L152
Useful? React with 👍 / 👎.
…ds time (#3009) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: GPT-5 <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d97da97cc7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type Transport = S; | ||
|
|
||
| fn spawn(&self, machine: moq_net::runtime::Machine<Self>) { | ||
| *self.slot.lock().unwrap() = Some(machine); |
There was a problem hiding this comment.
If cloned Inline handles are used to connect or accept a second session before take() is called, this assignment silently drops the first machine and replaces it. The first returned Session then remains open but has no protocol driver, so it never makes progress or processes shutdown. Make the runtime single-use by construction, or at minimum reject an occupied slot instead of overwriting it.
AGENTS.md reference: AGENTS.md:L158-L163
Useful? React with 👍 / 👎.
1 participant
Summary
moq_net::runtime: an explicitly injectedRuntimetrait (associatedTransportandTimertypes,timer(), a defaultednow(), andspawn) that supplies the two things a session cannot do alone: run its protocolruntime::Machineand wake its timers. No thread-locals, no globals, no ambient executor, and noSendbounds anywhere in the trait: pinning one transport type per runtime lets each implementation know the concrete machine it spawns, so a work-stealing tokio runtime and a future pinned!Sendio_uring runtime are both expressible with no feature flags.Client::connect(runtime, transport)/Server::accept(runtime, transport)now return the plainSession; the publicDrivertype is removed and the machine goes straight toRuntime::spawn.kio::time.moq_tokio::runtime::Runtime<S>(tokio spawn + tokio sleeps;now()reads tokio's pausable clock) andruntime::Inline<S>(hands the machine back for callers that drive it in place; the relay's WebSocket handler keeps its inline drain semantics through it), browser runtimes in moq-wasm (public, for direct Rust-on-wasm users) and moq-ffi, andmoq_net::runtime::Testbehind the newtest-runtimefeature: a deterministic virtual-clock runtime with explicitadvance/advance_to_timer/tickand no auto-advance.tokio::time::pause()semantics through acfg(test)tokio-backed runtime, so the 67 paused tests migrate mechanically rather than semantically.This is PR 1 of the plan on #2875 (#2875 (comment)): the moq-net prerequisite for the thread-per-core io_uring runtime.
Two deviations from that plan, discovered during implementation:
Runtimekeeps anow()(defaulted to the real clock). The plan said no clock on the trait, but relative arming (now + interval) and stamp-derived deadlines (linger) must read the virtual clock under a virtual-time test, or anadvance()leaves every subsequently armed instant in the past (the bandwidth sampler would fire on every poll, forever). The default keeps real runtimes on the std clock; onlyTestoverrides it.kio::timefor now. The model layer also owns deadlines (the origin driver's serve/linger machinery) and ~80 model tests manipulate the paused clock, so the kio 0.6time/tokioremoval moves to the follow-up PR that migrates the model andorigin::Driver. This PR converts the session side, which is everything a session runtime needs.Public API changes
dev):Client::connect/Server::acceptgain a leadingruntimeparameter and returnSessioninstead of(Session, Driver);Driveris removed;Requestgains a runtime parameter; newpub mod runtime(Runtime,Timer,Deadline,Machine,Instant, and feature-gatedTest/Never); newtest-runtimefeature.pub mod runtime(Runtime<S>,Inline<S>,Timer). Its ownClient/Serversurfaces are unchanged; consumers of moq-tokio need no changes.pub mod runtimefor direct Rust-on-wasm users ofmoq_net::Client.Wire behavior changes
None. No draft, JS, or cross-language synchronization is required;
doc/lib/rs/env/wasm.md(the one doc example using moq-net directly) andrs/CLAUDE.mdare updated.Test plan
nix develop --command just fixnix develop --command just checknix develop --command just test(3,478 passed, 2 skipped)runtime::Testtimer semantics (fire/re-arm/disarm/wake/advance-to-timer, and a regression for the re-arm-after-advance hazard that motivatednow()).(Written by Claude Fable 5)
🤖 Generated with Claude Code
https://claude.ai/code/session_01BCxzHiGxN5qi8Gd687nsgm