GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — feat(arty): add runtime hook API by martintmk · Pull Request #966 · microsoft/oxidizer Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## main #966 +/- ## ========================================== Coverage 100.0% 100.0% ========================================== Files 798 136 -662 Lines 68902 8858 -60044 ========================================== - Hits 68902 8858 -60044
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
234c703 to
084d7ee
Compare
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Introduce typed worker lifecycle hooks for startup and async panic handling. Keep hook data worker-local, route live registration through system commands, and leave blocking panics as join errors. Co-authored-by: Copilot App <[email protected]>
3d2db3e to
2dc6b7d
Compare
After init succeeds, retain a private OnStart hook with a relocated scheduler so future dynamically scaled workers enter the same Tokio context. Co-authored-by: Copilot App <[email protected]>
Keep private OnStart state for future dynamic workers and cover the Arty panic fallback that runs without an installed scheduler. Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Critical hook-registration bugs can reorder panic handlers or trigger a reentrant mutable-borrow panic, selecting the wrong panic action or interrupting delivery of the original panic result.
This PR adds typed runtime hooks to Arty, replacing async panic-policy configuration with callbacks and integrating hook registration into worker startup and runtime control.
Changes:
HookFn, OnStart, and OnPanic, with worker-local data and registration through builder and control APIs.| File | Description |
|---|---|
crates/arty/tests/runtime_hooks.rs |
Add public hook contract tests. |
crates/arty/tests/runtime_contracts.rs |
Use the ignore hook in panic tests. |
crates/arty/tests/public_paths.rs |
Check public hook types and traits. |
crates/arty/tests/panic/self_wake.rs |
Migrate self-wake tests to hooks. |
crates/arty/tests/panic/policy.rs |
Update async and blocking panic contracts. |
crates/arty/tests/panic_support/mod.rs |
Configure shared test runtimes with hooks. |
crates/arty/tests/join_failures.rs |
Replace legacy panic configuration. |
crates/arty/tests/construction.rs |
Exercise hook-based runtime construction. |
crates/arty/tests/behavior_contracts.rs |
Use hooks in runtime behavior tests. |
crates/arty/src/task/scheduler.rs |
Update the blocking-panic documentation reference. |
crates/arty/src/task/runtime_scheduler.rs |
Update panic documentation and test configuration. |
crates/arty/src/task/preparation/tests.rs |
Revise blocking-panic preparation tests. |
crates/arty/src/task/preparation/blocking.rs |
Remove shutdown decisions for blocking panics. |
crates/arty/src/task/handle/error.rs |
Reference hooks in panic-error documentation. |
crates/arty/src/task/handle/channel.rs |
Remove an unused shutdown accessor. |
crates/arty/src/runtime/worker/blocking/worker.rs |
Remove blocking-worker panic-policy state. |
crates/arty/src/runtime/worker/blocking/panic_policy.rs |
Delete the blocking panic-policy type. |
crates/arty/src/runtime/worker/blocking/mod.rs |
Remove panic-policy module wiring. |
crates/arty/src/runtime/worker/asynchronous/worker.rs |
Resolve panics through hooks and process registrations. |
crates/arty/src/runtime/worker/asynchronous/thread.rs |
Relocate hooks and invoke startup callbacks. |
crates/arty/src/runtime/worker/asynchronous/local_worker.rs |
Store, invoke, and clear worker-local hooks. |
crates/arty/src/runtime/worker/asynchronous/command.rs |
Add a hook-registration command. |
crates/arty/src/runtime/testing.rs |
Centralize runtime test helpers. |
crates/arty/src/runtime/mod.rs |
Export hooks and revise runtime defaults. |
crates/arty/src/runtime/hook/registry.rs |
Store ordered callback collections. |
crates/arty/src/runtime/hook/on_start.rs |
Define the worker-start event. |
crates/arty/src/runtime/hook/on_panic.rs |
Define panic events and response selection. |
crates/arty/src/runtime/hook/mod.rs |
Expose and document the hook API. |
crates/arty/src/runtime/hook/hook_fn.rs |
Pair callbacks with cloneable, relocatable data. |
crates/arty/src/runtime/handle.rs |
Update runtime-handle tests and helpers. |
crates/arty/src/runtime/dispatch/core.rs |
Distribute registrations across live workers. |
crates/arty/src/runtime/dispatch/client.rs |
Forward hook registrations to dispatch. |
crates/arty/src/runtime/control.rs |
Add late hook registration. |
crates/arty/src/runtime/config/runtime_config.rs |
Replace panic policy with hook storage. |
crates/arty/src/runtime/config/panic_policy.rs |
Delete the legacy public panic policy. |
crates/arty/src/runtime/config/mod.rs |
Remove the legacy policy export. |
crates/arty/src/runtime/builder.rs |
Add hook registration during construction. |
crates/arty/src/runtime/bootstrap/startup.rs |
Clone configured hooks for each worker. |
crates/arty/examples/arty_hooks.rs |
Demonstrate panic-hook registration. |
crates/arty/docs/PANICS.md |
Explain hook-based panic handling. |
crates/arty/Cargo.toml |
Register the hook example. |
crates/arty/AGENTS.md |
Update async panic-handling guidance. |
crates/arty_tokio/src/lib.rs |
Document intended support for future workers. |
crates/arty_tokio/src/initialization.rs |
Register a startup hook for Tokio context installation. |
crates/arty_tokio/Cargo.toml |
Add relocation and mutation-test dependencies. |
crates/arty_macros_impl/src/lib.rs |
Generate hook-based panic configuration. |
crates/arty_executor/tests/lifecycle/completion_lifecycle.rs |
Cover dynamic panic decisions. |
crates/arty_executor/src/task.rs |
Resolve panic actions before publishing completion. |
crates/arty_executor/src/panic_policy.rs |
Add dynamic panic resolution. |
crates/anyspawn/tests/arty.rs |
Migrate Arty integration tests to hooks. |
Cargo.lock |
Record added Tokio integration dependencies. |
Review effort: Balanced
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| if let Some(source) = &source { | ||
| LocalWorker::install_hook(hook.clone(), Some(source)); |
| let current = current.expect("a running worker retains its scheduler while processing hook registration"); | ||
| current.thread.clone() | ||
| }); | ||
| HOOKS.with_borrow_mut(|hooks| { |
|
|
||
| [[example]] | ||
| name = "arty_hooks" | ||
| required-features = ["rt"] |
| /// processing timers. The function runs on a separate pool thread. | ||
| /// A panic in this function returns an error through [`JoinHandle`] and | ||
| /// follows the configured [`PanicPolicy`](crate::runtime::PanicPolicy). | ||
| /// follows the configured [`OnPanic`](crate::runtime::hook::OnPanic) hook. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Hook registration can violate callback order or trigger mutable-borrow panics during callback execution, and the new example lacks its required macros feature gate.
In code that hasn't changed since last review
crates/arty/tests/runtime_hooks.rs:173
These external registrations are not acknowledged before the test submits a panicking task. In worker.rs, a worker can check the system queue before registration arrives, then receive and poll the task in the same cycle with the old panic response. Perform both registrations on the worker, where on applies synchronously, and wait for each registration call to finish before submitting the panic task.
crates/arty/tests/runtime_hooks.rs:252
The counter is read before either worker is known to have processed the late registration, so an incorrect OnStart replay can go unnoticed. Shutdown can also discard the queued registration. Add an acknowledgement of registration processing on both workers before checking the counter; an ordinary task is not a sufficient barrier because prepare_cycle can receive it after checking the system queue.
crates/arty/src/task/handle/error.rs:27
The default-shutdown description also needs the new async/blocking distinction. Blocking failures return this same JoinError, but task/preparation/blocking.rs no longer starts shutdown or invokes OnPanic. Limit the default shutdown and ignore-hook statements to async factories and polls, and state the blocking behavior explicitly.
crates/arty/src/task/runtime_scheduler.rs:205
This still promises hook handling for blocking panics, but task/preparation/blocking.rs:35–39 now only reports them through their join handles. OnPanic explicitly excludes blocking functions. Update this method's contract so callers do not rely on a hook to observe blocking failures or request shutdown.
Review effort: Balanced
Sorry, something went wrong.
There was an error while loading. Please reload this page.
At least 2 approving reviews are required to merge this pull request.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.