Add optional execution-cancellation breakpoint - #1900
Open
gwenaskell wants to merge 9 commits into
Open
Conversation
gwenaskell
added a commit
that referenced
this pull request
Aug 21, 2026
gwenaskell
changed the title
Adds an execution_timeout feature that lets embedders bound how long a VRL program is allowed to run. When enabled and a timeout is set via Runtime::set_timeout, every expression resolution checks a deadline (throttled to once every 1024 calls) and panics if it has passed. This is a no-op when the feature is disabled, which is the default. The check is inserted at the single point every expression in a program flows through (Expr::resolve), so it also covers the interpreter's only looping construct: stdlib iteration functions (for_each, map_values, filter, etc.) re-enter this dispatch once per closure invocation.
… timeout Renames execution_timeout to execution_cancellation. Instead of the compiler tracking an internal deadline, Runtime::set_cancellation_flag now takes an Arc<AtomicBool> the embedder owns and can flip from any thread (e.g. from their own timer, on client disconnect, on shutdown). The breakpoint hook is renamed to Context::cancel_breakpoint to reflect that it only checks this flag. Dropping the internal deadline also removes the need to throttle clock reads: an atomic load is cheap enough to check on every expression resolution.
gwenaskell
force-pushed
the
yoenn.burban/vrl-execution-timeout-breakpoint
branch
from
August 21, 2026 13:49
6a78dca to
8be0191
Compare
Reorders the std::sync imports to match rustfmt's grouping, and drops the unnecessary raw-string hashes flagged by clippy::pedantic's needless_raw_string_hashes now that CI runs against the 1.95 toolchain.
Puts #[cfg(feature = "execution_cancellation")] on the call site in Expr::resolve instead of relying on an empty no-op impl of Context::cancel_breakpoint to absorb the disabled case. Drops that no-op impl now that it's unreachable. Also drops the changelog fragment: this is an opt-in, embedder-only Cargo feature with no user-facing effect on VRL scripts themselves.
gwenaskell
added
the
no-changelog
gwenaskell
marked this pull request as ready for review
August 24, 2026 09:32
dd-sebastien-lb
approved these changes
Aug 24, 2026
It only stayed race-free by relying on debug-profile interpreter speed outpacing a fixed 5ms sleep — CI runs cargo test without --release, so this held today, but it's a timing assumption rather than a guarantee, and AtomicBool cross-thread visibility isn't logic this crate owns. for_each_loop_panics_when_already_cancelled already covers the actual wiring (check_cancellation is consulted and panics) deterministically.
![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: d9c9532cdb
ℹ️ 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".
check_cancellation now raises std::panic::panic_any(Cancelled) instead of assert!'s string payload, so a caller wrapping Runtime::resolve in catch_unwind can downcast to distinguish an intentional cancellation from any other panic. Also trims the set_cancellation_flag/ cancel_breakpoint doc comments down to what a cold reader needs.
Addresses two review findings on the cancel_breakpoint call site: - the flag is only checked between expressions, so a single long-running or blocking stdlib call still runs to completion before the next check can fire - a RuntimeState caught mid for_each-closure cancellation shouldn't be reused, since the panic can unwind before closure::Runner's cleanup restores a shadowed outer variable Both are inherent to checking cancellation at expression-dispatch granularity rather than something this PR fixes.
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
3 participants
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.
Summary
execution_cancellationCargo feature (off by default) that lets an embedder abort a running VRL program from another thread.Arc<AtomicBool>and registers it viaRuntime::set_cancellation_flag(orRuntimeState::set_cancellation_flag). Flip it totruefrom anywhere — your own timeout timer, a client disconnect, a shutdown signal — and the program panics as soon as it's observed, rather than running to completion.Context::cancel_breakpoint, called once perExpr::resolve— the single dispatch point every expression in a program flows through. This also covers VRL's only loop construct: stdlib iteration functions (for_each,map_values,filter,reduce, ...) invoke their closure body through this same path once per element, so a script looping over a large/attacker-controlled collection is caught too.AtomicBool::load(Relaxed), cheap enough to check on every expression resolution with no throttling needed. Timing policy (how long is "too long") is entirely up to the embedder.Context::cancel_breakpointcompiles to an empty#[inline(always)]function — a true no-op.Why panic instead of a
TerminateerrorThis is meant as a hard safety net against runaway scripts, not a recoverable VRL-level error like
abort. Callers that want to turn this into a graceful failure can wrapRuntime::resolveinstd::panic::catch_unwind.Test plan
cargo test --features default --lib— 1851 tests pass, unchanged.cargo test --features default,execution_cancellation --lib— 1855 tests pass (4 new).cargo clippy --features default --lib -- -D warningsandcargo clippy --features default,execution_cancellation --lib -- -D warnings— clean.compiler::state::execution_cancellation_tests).for_eachloop (compiler::runtime::execution_cancellation_tests): one cancelled before it starts, one cancelled from a second thread mid-loop over 500,000 elements — both panic as expected. Ran the mid-execution test 20x to check for flakiness; stable.