use web-time for the resource-tracker clock so max_duration works on wasm32-unknown-unknown#554
use web-time for the resource-tracker clock so max_duration works on wasm32-unknown-unknown#554Butch78 wants to merge 2 commits into
Conversation
…wasm32-unknown-unknown
`LimitedTracker` enforces `ResourceLimits::max_duration` via `std::time::Instant`.
On `wasm32-unknown-unknown`, `Instant::now()` panics ("time not implemented on
this platform"), so any run configured with a duration limit aborts. monty's
wasm CI targets `wasm32-wasip1`, where `Instant` is implemented, so this path is
green upstream and the gap is invisible there.
Consumers that embed the `monty` crate directly as a `wasm32-unknown-unknown`
wasm-bindgen module (in-process, browser-side Python) can't currently arm a
duration limit without panicking — and `max_duration` is the only limit that
stops a non-allocating runaway such as `while True: pass` (`max_allocations` /
`max_memory` never fire on it).
Swap the tracker clock to `web_time::Instant`, a drop-in that re-exports
`std::time::Instant` verbatim on native and WASI and is backed by
`performance.now()` on `wasm32-unknown-unknown`. No behavior change on any
currently-supported target; all resource-limit tests pass unchanged and
`wasm32-wasip1` still compiles.
There was a problem hiding this comment.
Pull request overview
This PR makes Monty’s ResourceLimits::max_duration enforcement work on wasm32-unknown-unknown by switching the resource tracker clock from std::time::Instant (which panics on that target) to web_time::Instant (which is backed by performance.now() in browsers and re-exports std::time::Instant on native/WASI).
Changes:
- Replace
std::time::Instantusage in the resource tracker withweb_time::Instantto avoidInstant::now()panics on browser wasm builds. - Add the
web-timedependency to themontycrate. - Update
Cargo.lockto include the new dependency.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/monty/src/resource.rs | Switches the tracker clock to web_time::Instant so duration limits don’t panic on wasm32-unknown-unknown. |
| crates/monty/Cargo.toml | Adds the web-time dependency to support a portable monotonic clock. |
| Cargo.lock | Records the resolved web-time dependency in the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // `web_time::Instant` is `std::time::Instant` on native and WASI targets, and a | ||
| // `performance.now()`-backed clock on `wasm32-unknown-unknown`, where the std | ||
| // `Instant::now()` panics ("time not implemented on this platform"). This keeps | ||
| // the `max_duration` time limit working when `monty` is embedded directly in a | ||
| // browser wasm module. Time behavior on all currently-supported targets is | ||
| // unchanged (it re-exports std verbatim there). | ||
| use web_time::Instant; |
| # Portable monotonic clock: a drop-in for `std::time::Instant` that re-exports | ||
| # std unchanged on native and WASI, and is backed by `performance.now()` on | ||
| # `wasm32-unknown-unknown` (where `std::time::Instant::now()` panics). Keeps the | ||
| # `max_duration` resource limit usable when `monty` is embedded directly in a | ||
| # browser wasm module rather than run over WASI. | ||
| web-time = "1.1" |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
This seems reasonable to me. Should we make it target-specific dependency just to avoid it where it's unnecessary? I'm also curious about your use-case. It sounds like the work I just did in #525 will probably be relevant for you, which uses |
Per review: web_time::Instant is only needed where std::time::Instant panics, i.e. wasm32-unknown-unknown. Gate the dependency and the import on that target so every other target (native, WASI) uses std directly and never pulls web-time into its graph. Verified: web-time is absent from the native and wasm32-wasip1 dependency graphs and present only under wasm32-unknown-unknown; both compile and all resource-limit tests still pass.
|
Good tip, I made it target specific! On the use-case @davidhewitt : I embed the The results have been amazing, I'm actually lazy loading the binary and even with a bad connection It works nearly instantly I'm also run a second tier for the harder cases: I'm experimenting with And yes, #525 looks very relevant. The I am are currently using the in-process wasm-bindgen embedding, so moving to that shape is a larger change we are weighing; this fix unblocks the cooperative time budget on our current architecture in the meantime. I had a call with @samuelcolvin and he recommended focusing on browser first, I'm very close to that and would love to have call with you if wanted to see a demo @davidhewitt I'm hoping to soon have a beta out so people can try :) |
LimitedTrackerenforcesResourceLimits::max_durationviastd::time::Instant(crates/monty/src/resource.rs). Onwasm32-unknown-unknown,Instant::now()panics (time not implemented on this platform), so any run configured with a duration limit aborts. monty's wasm CI targetswasm32-wasip1, whereInstantis implemented, so this path is green upstream and the gap is invisible there.Use case
We embed the
montycrate directly as awasm32-unknown-unknownwasm-bindgen module for in-process, browser-side Python in a cell (not the WASI/worker path). There,max_durationis the only limit that stops a non-allocating runaway such aswhile True: pass—max_allocations/max_memorynever fire on it — and we can't arm it without panicking.Change
Swap the tracker clock to
web_time::Instant, a drop-in that re-exportsstd::time::Instantverbatim on native and WASI and is backed byperformance.now()onwasm32-unknown-unknown. No behavior change on any currently-supported target.Verification
timeout_*case andsuspension_time_does_not_count_toward_max_duration).wasm32-wasip1(supported wasm target) and native compile clean;clippy -D warningsclean.web_time::Instantconfirmed compiling forwasm32-unknown-unknownwith monty's exact usage (Instant::now(),Cell<Option<Instant>>,.elapsed()).I see the time code is in flux for the 0.19 subprocess model (#483) — happy to retarget or adjust timing to suit.
Summary by cubic
Switch the resource tracker to
web-time::Instantsomax_durationworks onwasm32-unknown-unknownwithout panicking. No behavior change on native orwasm32-wasip1; restores duration limits for browser-embeddedmonty.Bug Fixes
std::time::Instantwithweb_time::Instantin the tracker to avoidInstant::now()panic onwasm32-unknown-unknown.while True: pass) in browser builds.Dependencies
web-timev1.1 as a target-gated dependency forwasm32-unknown-unknown; importweb_time::Instantonly there and usestd::time::Instanteverywhere else.Written for commit ba9ba9a. Summary will update on new commits.