Draft: asynchronous phase handler #237

ensh63 · 2026-01-09T00:58:10Z

Draft implementations of the following items are added to SDK:

asynchronous phase handler;
asynchronous subrequest;
type-safe request context;
typed storage for Pool.

Copilot

Pull request overview

This PR introduces asynchronous capabilities to the Rust SDK for nginx modules, including async phase handlers, subrequests, and type-safe request context management.

Key Changes:

Introduces AsyncHandler trait and supporting infrastructure for asynchronous HTTP request handling
Adds RequestContext trait for type-safe request-specific context data management
Implements TypeStorage trait for storing typed values in nginx memory pools
Refactors existing examples to use new HttpRequestHandler trait and add_phase_handler function

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/log.rs	Extracts connection to intermediate variable in logging macro
src/http/status.rs	Adds `From<HTTPStatus>` implementation for `ngx_int_t`
src/http/request_context.rs	Implements `RequestContext` trait for managing request-specific context
src/http/request.rs	Adds `HttpHandlerReturn` and `HttpRequestHandler` traits, new request methods
src/http/mod.rs	Adds async_request module exports (feature-gated)
src/http/conf.rs	Adds `HttpPhase` enum and `add_phase_handler` function
src/http/async_request.rs	Implements async handler infrastructure and subrequest builder
src/core/type_storage.rs	Implements `TypeStorage` trait for pool-based type storage
src/core/pool.rs	Adds `remove` method for cleaning up pool-allocated values
src/core/mod.rs	Exports new type_storage module
examples/*.rs	Refactors examples to use new handler registration pattern
examples/config	Adds async_request module configuration
examples/Cargo.toml	Adds async-related dependencies
Cargo.toml	Adds futures dependency
.github/workflows/nginx.yaml	Updates CI to load async_request module

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/http/request_context.rs

src/http/async_request.rs

src/core/type_storage.rs

src/core/pool.rs

xeioex · 2026-01-21T02:40:38Z

src/core/pool.rs

-        (*cln).data = value as *mut c_void;
+        // `data` points to the memory allocated for the value by `ngx_pool_cleanup_add()`
+        ptr::write((*cln).data as *mut T, value);
+        NonNull::new((*cln).data as *mut T)


If T is ZST
cln.data will be NULL
ptr::write((*cln).data as *mut T, value) - is fine as null is explicitly allowed when value is ZST.
but NonNull::new() will return None.

as a result allocate<T>() will return ptr::null_mut().

I do not know should we improve it or not, but probably we at least should document it.

xeioex · 2026-01-21T02:56:40Z

src/core/pool.rs

+    ///
+    /// Returns a typed pointer to the allocated memory if successful, or `None` if
+    /// allocation or cleanup handler addition fails.
+    pub fn allocate_unique<T: Sized>(&mut self, value: T) -> Option<&mut T> {


It will return None if the T is already allocated.

xeioex · 2026-01-21T02:58:38Z

src/core/pool.rs

+    /// This function is marked as unsafe because it involves raw pointer manipulation.
+    unsafe fn allocate_with_cleanup_unique<T: Sized>(&self, value: T) -> Option<NonNull<T>> {
+        if self.cleanup_lookup::<T>(None).is_some() {
+            return None;


This is a bit confusing, if this happen it looks like a logic error in code (not runtime error)
can we just panic here?

xeioex · 2026-01-22T02:33:33Z

src/http/async_request.rs

+use core::pin::Pin;
+use core::task::{Context, Poll};
+
+use crate::core::type_storage::*;


TypeStorage is absent, the commit is broken.

xeioex · 2026-01-23T02:20:38Z

src/http/async_request.rs

+        if ctx.launcher.is_none() {
+            Err(AsyncHandlerError::NoAsyncLauncher)
+        } else if ctx.launcher.as_ref().unwrap().is_finished() {
+            let rc = futures::executor::block_on(ctx.launcher.take().unwrap());


It seems, we use futures just for this one line.
Can we write something like fast_poll_ready() manually and remove the dependency?

Satifying fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> is not easy, but still doable. Using RawWakerVTable and RawWaker::new() with stub methods.

I wander is it something worth doing and what is the trade-off.

xeioex · 2026-01-23T02:23:30Z

src/http/async_request.rs

+    /// The return type of the asynchronous worker function.
+    type ReturnType: HttpHandlerReturn;
+    /// The asynchronous worker function to be implemented.
+    fn worker(request: &mut Request) -> impl Future<Output = Self::ReturnType>;


I wonder what prevents us from using async fn worker() here? Given that is should be available since 1.75.

xeioex · 2026-01-23T23:02:19Z

.github/workflows/nginx.yaml


  NGX_TEST_FILES: examples/t
  NGX_TEST_GLOBALS_DYNAMIC: >-
    load_module ${{ github.workspace }}/nginx/objs/ngx_http_async_module.so;


As we have native async support now, we should remove half-working ngx_http_async_module example and remove tokio dependency.

xeioex · 2026-01-24T00:21:05Z

examples/t/async_request.t

+
+EOF
+
+$t->write_file('index.html', '');


The file is not needed here.

Also, more tests are needed, including negative ones.

xeioex · 2026-01-24T00:56:04Z

src/http/async_request.rs

+}
+
+impl<'sr> core::future::Future for AsyncSubRequest<'sr> {
+    type Output = (ngx_int_t, Option<&'sr Request>);


Why not to make a Result<> value here too?

diff --git a/examples/async_request.rs b/examples/async_request.rs index a616b7d3..507268a3 100644 --- a/examples/async_request.rs +++ b/examples/async_request.rs @@ -63,20 +63,12 @@ impl AsyncHandler for SampleAsyncHandler { let request_ptr: *mut ngx_http_request_t = request.as_mut(); let fut = AsyncSubRequestBuilder::new("/proxy") - //.args("arg1=val1&arg2=val2") + .args("arg1=val1&arg2=val2") .in_memory() .waited() .build(request)?; - let subrc = fut.await; - - ngx_log_error!(nginx_sys::NGX_LOG_INFO, log, "Subrequest rc {}", subrc.0); - - if subrc.0 != nginx_sys::NGX_OK as _ || subrc.1.is_none() { - return Err(SampleAsyncHandlerError::from(subrc.0)); - } - - let sr = subrc.1.unwrap(); + let sr = fut.await?; ngx_log_error!( nginx_sys::NGX_LOG_INFO, diff --git a/nginx-src/nginx b/nginx-src/nginx index 95078974..481d28cb 160000 --- a/nginx-src/nginx +++ b/nginx-src/nginx @@ -1 +1 @@ -Subproject commit 95078974cfe998ec4095ac2c504c5c92bbc9a326 +Subproject commit 481d28cb4e04c8096b9b6134856891dc52ecc68f diff --git a/src/http/async_request.rs b/src/http/async_request.rs index 83ac81b4..82355e87 100644 --- a/src/http/async_request.rs +++ b/src/http/async_request.rs @@ -317,7 +317,7 @@ impl<'sr> AsyncSubRequest<'sr> { } impl<'sr> core::future::Future for AsyncSubRequest<'sr> { - type Output = (ngx_int_t, Option<&'sr Request>); + type Output = Result<&'sr Request, ngx_int_t>; fn poll( self: Pin<&mut Self>, @@ -332,7 +332,7 @@ impl<'sr> core::future::Future for AsyncSubRequest<'sr> { ngx_cycle_log().as_ptr(), "Subrequest not created" ); - return core::task::Poll::Ready((nginx_sys::NGX_ERROR as _, None)); + return core::task::Poll::Ready(Err(nginx_sys::NGX_ERROR as _)); } if this.rc.is_none() { @@ -341,8 +341,8 @@ impl<'sr> core::future::Future for AsyncSubRequest<'sr> { } // let request: &Request = unsafe { Request::from_ngx_http_request(this.sr.take().unwrap()) }; - let rc = this.rc.unwrap(); + // let rc = this.rc.unwrap(); // ngx_log_debug_http!(request, "subrequest poll: ready({rc})"); - core::task::Poll::Ready((rc, Some(this.sr.take().unwrap()))) + core::task::Poll::Ready(Ok(this.sr.take().unwrap())) } }

ensh63 added 2 commits January 8, 2026 10:44

feat: trait for HTTP request handler

6b11068

chore: make clippy tool happy

520b4ff

ensh63 force-pushed the shirykalov/async branch from 4a7588b to 315d8e3 Compare January 9, 2026 01:01

ensh63 requested review from bavshin-f5, Copilot and xeioex January 9, 2026 01:10

Copilot AI reviewed Jan 9, 2026

View reviewed changes

src/http/request_context.rs Outdated Show resolved Hide resolved

src/http/async_request.rs Show resolved Hide resolved

src/http/async_request.rs Show resolved Hide resolved

src/core/type_storage.rs Outdated Show resolved Hide resolved

src/core/pool.rs Outdated Show resolved Hide resolved

feat: trait for module request context

856fc07

ensh63 force-pushed the shirykalov/async branch from 315d8e3 to 78a97e6 Compare January 9, 2026 23:42

ensh63 added 3 commits January 9, 2026 15:48

feat: typed storage in pool

ae4ea93

feat: async phase handler

eededf3

feat: async subrequest

27338b6

ensh63 force-pushed the shirykalov/async branch from 78a97e6 to 27338b6 Compare January 9, 2026 23:48

xeioex requested changes Jan 24, 2026

View reviewed changes


		EOF

		$t->write_file('index.html', '');

Draft: asynchronous phase handler #237

Are you sure you want to change the base?

Draft: asynchronous phase handler #237

Conversation

ensh63 commented Jan 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants