Repository navigation
quest(m2): add leak-free poll forms for the QUIC driver's waits #6329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e6e58de
quest(m2): add leak-free poll forms for the QUIC driver's waits
zshenker 2bb0b57
quest(m2): driver-poll builds on the merged stopped waiter map
zshenker 65a4bfe
Merge remote-tracking branch 'origin/main' into pr-6329
kixelated 6978f18
quest(m2): settle driver-poll's shape and narrow its index entry
kixelated File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| # [M] The QUIC driver's waits poll without leaking | ||
|
|
||
| ## Goal | ||
|
|
||
| The stream, datagram, close, and accept waits in `moq_tokio::quic::driver` | ||
| have `poll_*` forms that leak nothing: `Connection::poll_open_uni`, `poll_open_bi`, `poll_accept_uni`, | ||
| `poll_accept_bi`, `poll_recv_datagram` and `poll_closed`, | ||
| `SendStream::poll_closed` (today's `stopped`), and `Endpoint::poll_accept`. | ||
| `SendStream::poll_acked`, which [poll_acked](/quest/m2/quic-ack-hook.md) | ||
| adds, uses the same mechanism. The names follow moq-uring's driver, which | ||
| already follows moq-net's `transport::poll` traits; return types stay the | ||
| driver's own. A waiter that polls once and is dropped leaves nothing alive: | ||
| no live registration and no task. | ||
|
|
||
| ## Plan | ||
|
|
||
| The driver's waits are futures over `tokio::sync::Notify`: the connection's | ||
| `Shared` fields, the per-stream `stopped` map, and the endpoint's `incoming`. | ||
| `Notify` cannot hold a bare waker, so a poll-based caller has to keep the | ||
| future, and moq-tokio's `transport::Session` adapter boxes one per pending | ||
| operation. [Drop web-transport-trait](/quest/m2/drop-web-transport-trait.md) | ||
| wants a native `poll::Session` on the QUIC backend, and | ||
| [poll_acked](/quest/m2/quic-ack-hook.md) wants a real poll method. Both need | ||
| these. | ||
|
|
||
| The trap is a stateless `&self` poll that pushes the caller's waker into a | ||
| shared list. Nothing removes the entry when its caller gives up, so it lives, | ||
| holding its task, until the next signal; for `poll_closed` that is the end of | ||
| the connection. A trial of that shape kept all 1000 abandoned `poll_closed` | ||
| waiters. Leak-free needs per-caller state that dies with the caller, as | ||
| tokio's `Notified` has, and as kio's `Park` and `WaiterList` give through | ||
| `Weak` entries. | ||
|
|
||
| Shape: poll forms take `&mut self, cx`, with a `kio::Park` in each handle. A | ||
| clone gets an empty park, so a registration belongs to the handle that made | ||
| it and dies with it. A handle that stops polling but stays alive keeps at most | ||
| its one registration until its next poll or the signal; that is the standard | ||
| poll contract, and it is bounded per handle. This is moq-uring's driver shape | ||
| and moq-net's `poll::Session` shape, so a native impl forwards in one line, | ||
| and kio stays private. | ||
|
|
||
| ```rust | ||
| impl Connection { | ||
| pub fn poll_accept_uni(&mut self, cx: &mut Context<'_>) -> Poll<Result<RecvStream, ConnectionError>>; | ||
| pub fn poll_open_uni(&mut self, cx: &mut Context<'_>, options: SendOptions<'_>) -> Poll<Result<SendStream, ConnectionError>>; | ||
| pub fn poll_closed(&mut self, cx: &mut Context<'_>) -> Poll<ConnectionError>; | ||
| } | ||
| impl SendStream { | ||
| pub fn poll_closed(&mut self, cx: &mut Context<'_>) -> Poll<Result<Option<VarInt>, StoppedError>>; | ||
| } | ||
| ``` | ||
|
|
||
| Rejected shapes: | ||
|
|
||
| - **`&self, &kio::Waiter`**, the convention in `rs/AGENTS.md`. Leak-free, | ||
| since the caller owns the waiter, but kio enters the driver's public | ||
| signatures, so a kio break is a driver break, and every `cx`-based caller | ||
| (moq-net's traits) holds a `Park` anyway. | ||
| - **A caller-owned `Wait` token** (`poll_closed(&self, cx, &mut Wait)`), | ||
| slab-backed and released on drop: a second waiter primitive beside kio's, | ||
| with its own slab and tests to carry. | ||
| - **A stateless shared list**: the leak above. | ||
|
|
||
| Mechanism: swap the `Notify`s, and the `stopped` map's entries, | ||
| for `kio::WaiterList`s woken where the `Notify`s are signalled today, under | ||
| the connection lock, and make the existing async forms thin wrappers. They can | ||
| keep `&self` by owning their waiter (`kio::wait`), so no caller changes. Keep | ||
| the existing guarantees, such as draining accepted streams and datagrams | ||
| from a closed connection before its error. The other `Notify` waits | ||
| (`handshake_confirmed`, `Endpoint::wait_idle`) may move to the same lists, | ||
| but get no poll form until a caller needs one. | ||
|
|
||
| kio becomes a direct dependency of the `driver` feature. It is already in the | ||
| driver-only tree through `moq-quic`'s moq-clock, which #6132 accepted (its | ||
| decision 4), so no crate is added, and it stays out of the public API. | ||
|
|
||
| Builds on the `stopped` waiter map as #6196 left it, which already records a | ||
| local reset. It blocks the driver half of [poll_acked](/quest/m2/quic-ack-hook.md). | ||
|
|
||
| The acceptance test is the measured leak: 1000 cloned handles that each poll | ||
| once and are dropped unresolved leave no live registration behind, checked before the | ||
| condition is ever signalled, so release on drop is what passes it, not the | ||
| next wake. Also, on paused time: two handles waiting on each condition both | ||
| wake; one handle polled 100 times holds one live registration; a clone's poll | ||
| leaves the original's registration in place. Report the public API: new poll | ||
| methods on `Connection`, `SendStream` and `Endpoint`, with the async forms | ||
| unchanged. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Drop web-transport-trait](/quest/m2/drop-web-transport-trait.md) - its native QUIC `poll::Session` calls these | ||
| - [poll_acked on moq-net's send stream](/quest/m2/quic-ack-hook.md) - adds `SendStream::poll_acked` in this shape | ||
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.
Uh oh!
There was an error while loading. Please reload this page.