Repository navigation
Core: pool-result ceiling (#88), multishot accept budget (#77), safe loop rebuild (#43) - #106
Merged
Merged
Conversation
WorkPort's result ring was sized by max_operations, because that was the only bound on what can sit in it: every pool-delivered operation holds an operation credit until delivery, and push is an assert!. queue_capacity does not bound it (a finished job leaves the queue while its result waits, and long jobs never enter the queue), so the ring could not simply be made smaller. A host that sets max_operations to 32 768 for I/O therefore paid for a 32 768-slot result ring its pool work would never fill. The accounting is now separate, as the correction on #88 asks. PoolConfig gains max_undelivered (default 4096): a per-loop ceiling on operations that deliver through the ring - blocking jobs of both occupancy classes, pool lookups, native typed file requests and external waits. The driver admits each one against it before anything is created, refusing with ResourceLimit exactly like a full queue_capacity, charges the operation, and releases the credit when the operation retires, which is after its result has been popped from the ring. The ring is sized by min(max_undelivered, max_operations), so the assert's invariant now holds against the new counter: the ring's occupancy never exceeds the charged operations, and those never exceed its capacity. A zero ceiling is InvalidInput. max_undelivered is per loop, so it is left out of the process-wide pool configuration check: loops that differ only there share one pool. Tests: a contract scenario submits exactly max_undelivered jobs of both classes without turning, checks that one more blocking job, long job and external wait are each refused with ResourceLimit, lets every result land in the ring (filling it to capacity) and then delivers each exactly once, twice over to prove delivery returns every credit. It fails without the admission check. A unit test checks the ring is sized by the ceiling, never above max_operations, and that a zero ceiling is rejected. Breaking: PoolConfig has a new public field, so struct literals without ..Default::default() no longer compile. Closes #88
A multishot accept holds one handle reservation, so it was protected for one connection at a time, but one poll can hand it up to events_per_turn connections. Once the loop reached its ceiling mid-batch, the rest were accepted from the kernel and then destroyed, completing with ResourceLimit - the behaviour #75 removed for single-shot accepts. Backend::poll now takes a Budget. Its only field, accepts, is the number of connections multishot accepts may deliver in this poll: every free handle slot that no single-shot accept or handle receive has reserved. Each nonterminal Accepted/PipeAccepted event spends one. That is exactly what the core can house, so a batch can no longer outrun the ceiling. The budget throttles only multishot accepts, per source, as the issue requires for DESIGN section 10 rule 3: reads, writes, timers, posts, single-shot accepts and every other source fill the event vector exactly as before, so the fairness contract is untouched. A multishot accept out of budget takes nothing more from the kernel and is parked in a per-backend throttled list, which is neither runnable work (has_work) nor a reason to end the wait, so a turn at the ceiling blocks for its timeout instead of spinning (rule 4a). The first poll with a positive budget resumes it. Per backend: - epoll/kqueue (shared Unix engine), WASI 0.2, WASI 0.3: the listener's head accept is skipped and parked; readiness stays cached, so the connection stays in the backlog and is accepted when the listener resumes. WASI 0.2 also stops subscribing a parked listener's level-triggered pollable, which would otherwise end every wait at once. - IOCP: a parked accept is not re-armed with AcceptEx, and a connection an already-armed AcceptEx completed stays in the operation until it can be housed; cancellation still reaches a parked operation. - web: has no listener; it takes the parameter and ignores it. In the core, the multishot reservations are pooled rather than owned per operation. With per-operation ownership, two armed listeners could each hold a slot while one of them spent both within the budget, and the second delivery would then find its slot promised to the other listener and fail. A multishot delivery now spends any pooled reservation and the pool is refilled as far as the ceiling allows; the budget (free slots minus single-shot reservations) counts exactly the deliveries that can then succeed. Test: a new contract scenario floods a multishot accept on a loop with room for three connections (events_per_turn is 256), asserts no accept ever completes with an error and no more than three are live, that turns at the ceiling deliver nothing and block (at most 10 turns in 100 ms), and that every one of 12 clients is then accepted as slots are freed - none was destroyed. It fails without the budget (ResourceLimit completions). The existing fairness (timer_backlog_io_and_post_progress, sustained_posts_idle_io) and no-spin (no_spin, quiet_deadline_accounting) contract tests pass unchanged on macOS/kqueue. epoll, IOCP, WASI 0.2/0.3 and web were compile-checked (clippy --target) only on this host. Breaking: Backend::poll has a new parameter (the internal backend contract). Closes #77
A loop's Config is fixed at construction, so raising a profile means building a new loop, and dropping the old one closes its WorkPort: a pool job already in flight runs to the end and its completion is discarded. Perry works around this by always submitting pool work at its network profile. Of the two fixes #43 offers, growing a live loop in place is not feasible without redesigning structures other threads or the kernel hold: the post and result rings are lock-free and written by other threads (their capacity is their mask and their backpressure bound), and the Windows OVERLAPPED slab is contiguous because the kernel holds its addresses; max_operations and max_handles also size storage in every backend. So this takes the second option and makes replacement safe and loud. Driver::rebuild(config) builds a new loop in place, but only when the old one owes nothing a new loop could not deliver: no handle, no operation in flight (pool jobs, pool lookups, typed file requests and external waits included - every one is counted in `outstanding` until its completion is handed to the host), no queued completion, no result in the ring and no queued post. Otherwise it returns WouldBlock and changes nothing: the host turns until the work is delivered and retries, the same contract as detach. A config that Driver::new rejects fails the same way. Handles, notifiers, posters and the integration primitive from before belong to the old loop, as documented. Test: the issue's acceptance scenario, adapted to the option taken. A small profile loop submits a pool job that blocks on a gate; an upgrade to a larger profile is refused with WouldBlock while the job runs, and again while its result is waiting in the ring; the job's completion is then delivered exactly once (and nothing follows it); the upgrade succeeds, the new loop holds 256 handles where the old held 4, still refuses a rebuild while handles are live, runs pool work, and rebuilds again once drained. Closes #43
|
Warning Review limit reachedNext included review available in 11 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (15)
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 |
Review of #106: when the budget parks a multishot accept whose stream read has already fired, run_ready skips execute(), and execute() -> finish_wait() is the only place that removes the waitable from the wait-set. The accept's outcome is already held in `code`, so the membership serves nothing, and if the host re-delivered the event each wait at the ceiling would return at once: a turn loop that produces nothing (rule 4a). schedule() now removes that waitable when it parks the listener. This runs no outcome side effect and loses no connection: the result stays in `code`, and execute() consumes it when a positive budget resumes the listener, where finish_wait()'s own removal is a harmless repeat (join to 0 is idempotent). A parked accept can only be in one of two states, not yet started (no waitable) or fired (outcome in `code`), because a started, unfired read clears `ready`, so nothing needs to rejoin the set. On wasmtime 46 the stream-read event was delivered once even without this change (checked with instrumentation), so no spin was observed there. The fix keeps the backend from depending on that host behaviour. Test: a new contract scenario reaches this path deliberately. Two listeners share the ceiling; both accepts start waiting on empty backlogs, the second listener's connections take every free slot, and a client then connects to the first, so its read fires with no slot to house the connection. The test asserts turns at the ceiling deliver nothing and block (at most 20 turns in 200 ms, one native wait each), and that freeing one slot delivers exactly that connection. It runs on native and Windows, and on WASI 0.2 and 0.3 together with the existing multishot ceiling test, which tests/wasi.rs did not run before.
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
1 participant
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.
Three core fixes, one commit each.
#88: undelivered pool results get their own ceiling
WorkPort's ring was sized bymax_operations, because every pool-delivered operation keeps an operation credit until delivery and a full ring hits anassert!. As the correction comment on #88 asks, the accounting is now separate.PoolConfig::max_undelivered(default 4096, per loop, left out of the process-wide pool config check) caps the operations that deliver through the ring: blocking jobs of both classes, pool lookups, native typed file requests and external waits. Each one is admitted at submission (ResourceLimit, likequeue_capacity) and releases its credit when it retires, which happens after its result is popped. The ring is sized bymin(max_undelivered, max_operations), so the assert's invariant now holds against the new counter. A contract test fills the ring to exactly its capacity with results nobody has delivered yet, checks that each kind of submission is refused past the ceiling, then delivers every result exactly once (two rounds). A unit test checks the ring's size.Closes #88
#77: a multishot accept can no longer outrun the handle ceiling
Backend::pollnow takes aBudget { accepts }: the free handle slots that no single-shot accept or handle receive holds. Each connection a multishot accept delivers spends one. The budget throttles only multishot accepts, per source. Every other source fills the event vector as before, so DESIGN §10 rule 3 fairness holds. A multishot accept that is out of budget takes nothing more from the kernel and is parked outside the runnable set, so a turn at the ceiling blocks for its timeout instead of spinning (rule 4a). The accept resumes at the first poll whose budget is positive. This is implemented on epoll/kqueue (shared Unix engine), WASI 0.2 (which also unsubscribes the parked listener's level-triggered pollable), WASI 0.3 and IOCP (no re-arm; a connection that has already completed is held in the operation). Web takes the parameter and ignores it. The driver now pools multishot reservations so that two armed listeners sharing one budget cannot strand each other. The new contract test floods a multishot accept on a loop with room for 3 connections: no accept fails, turns at the ceiling deliver nothing and do not spin, and all 12 clients are accepted as slots free up. It fails without the budget. The existing fairness and no-spin contract tests pass unchanged.Closes #77
#43:
Loop::rebuildreplaces a loop without losing a pool completionGrowing a live loop in place (option 1) is not feasible without redesigning structures that other threads or the kernel hold: the lock-free post and result rings, and the contiguous Windows
OVERLAPPEDslab. This PR therefore takes option 2.Driver::rebuild(config)rebuilds the loop in place, but only when it owes nothing: no handle, no operation in flight (pool jobs included), no queued completion, ring result or post. Otherwise it returnsWouldBlockand nothing changes, the same contract asdetach. The acceptance test is adapted to this option. The upgrade is refused while a pool job runs, and again while its result waits in the ring. The job then delivers exactly once, after which the upgrade succeeds and the new profile is in effect.Closes #43
Breaking changes (pre-1.0)
PoolConfighas a new public fieldmax_undelivered. Struct literals without..Default::default()stop compiling.Backend::polltakes a newbudget: Budgetparameter (internal backend contract).ResourceLimitonce it hasmax_undeliveredresults outstanding.Verification
macOS/aarch64: fmt, workspace clippy, doc,
turnlooptests andturnloop-contracttests (--test-threads=1) pass. There are two exceptions, and both are FSEvents watch tests:filesystem::watch_backpressureandallocations::steady_watch_batches_allocate_nothing. They also fail on unmodifiedorigin/mainon this host (checked A/B, interleaved), so the cause is the host, not this PR. MSRVcargo +1.97.1 checkpasses. The epoll (x86_64-linux-gnu), IOCP (x86_64-pc-windows-msvc), WASI 0.2, WASI 0.3 (nightly-2026-09-07) and web backends were compile-checked only, withclippy --targetforturnloopandturnloop-contract. CI is the first place their tests run.