-
Notifications
You must be signed in to change notification settings - Fork 74
feat: minimal async runtime on top of the NGINX event loop #170
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a minimal async runtime on top of the NGINX event loop, providing task spawning and timer-based sleep support under a new async
feature.
- Adds a feature-gated
async_
module withspawn
andsleep
utilities - Implements a single-threaded scheduler (
spawn.rs
) that posts tasks via NGINX events - Provides
sleep
(sleep.rs
) as an async primitive usingngx_add_timer
/ngx_del_timer
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/lib.rs | Expose async_ module when async feature is set |
src/async_/spawn.rs | Scheduler implementation and spawn function |
src/async_/sleep.rs | sleep future implementation using NGINX timers |
src/async_/mod.rs | Re-export spawn and sleep from the async_ module |
Cargo.toml | Added optional async-task dependency and async feature flag |
Comments suppressed due to low confidence (4)
src/async_/spawn.rs:135
- [nitpick] There are no tests covering the
spawn
function and scheduling logic. Consider adding unit or integration tests to verify tasks are enqueued and executed correctly on the NGINX event loop.
pub fn spawn<F, T>(future: F) -> Task<T>
src/async_/sleep.rs:17
- [nitpick] The
sleep
function isn't covered by any tests. Add tests to ensure it completes after the expected delays, including edge cases around the maximum timer duration.
pub async fn sleep(duration: Duration) {
src/async_/sleep.rs:19
- The log macro uses a formatting placeholder
{duration:?}
without passing theduration
argument, so it will log the literal string. Change to:
ngx_log_debug!(timer.event.log, "async: sleep for {:?}", duration);
ngx_log_debug!(timer.event.log, "async: sleep for {duration:?}");
src/async_/sleep.rs:29
- Same issue here: the placeholder
{duration:?}
isn’t substituted. Update to:
ngx_log_debug!(timer.event.log, "async: sleep for {:?}", duration);
ngx_log_debug!(timer.event.log, "async: sleep for {duration:?}");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks excellent. Thank you for iterating on this!
This is a slightly more ergonomic variant of (*nginx_sys::ngx_cycle).log with safety invariants being documented.
59a4e5d
to
1a19a60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good.
This change introduces a general infrastructure for spawing async tasks on NGINX event loop. The only utility offered for now is a timer support via `ngx::async_::sleep`, with async IO and other scenarios being planned for future.
This change introduces a general infrastructure for spawing async tasks on NGINX event loop. The only utility offered for now is a timer support via
ngx::async_::sleep
, with async IO and other scenarios being planned for future.An incomplete connection IO example is available here: https://github.com/bavshin-f5/ngx-rust/tree/async