Skip to content

Commit 363b648

Browse files
committed
Partially revert ad677f6
It's true that ad677f6 creates two &mut Resolution on the same stack frame, and we need to avoid that.
1 parent 8bd5628 commit 363b648

File tree

1 file changed

+54
-55
lines changed

1 file changed

+54
-55
lines changed

src/async_/resolver.rs

Lines changed: 54 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88
//! See <https://nginx.org/en/docs/http/ngx_http_core_module.html#resolver>.
99
1010
use alloc::string::{String, ToString};
11+
use core::ffi::c_void;
1112
use core::fmt;
1213
use core::num::NonZero;
1314
use core::pin::Pin;
1415
use core::ptr::NonNull;
1516
use core::task::{Context, Poll, Waker};
1617

1718
use crate::{
19+
allocator::Box,
1820
collections::Vec,
1921
core::Pool,
2022
ffi::{
@@ -116,21 +118,14 @@ impl Resolver {
116118

117119
/// Resolve a name into a set of addresses.
118120
pub async fn resolve_name(&self, name: &ngx_str_t, pool: &Pool) -> Res {
119-
let mut ctx = ResolverCtx::new(self.resolver)?;
120-
ctx.name = *name;
121-
ctx.timeout = self.timeout;
122-
123-
Resolution::new(ctx, pool).await
121+
let mut resolver = Resolution::new(name, &ngx_str_t::empty(), self, pool)?;
122+
resolver.as_mut().await
124123
}
125124

126125
/// Resolve a service into a set of addresses.
127126
pub async fn resolve_service(&self, name: &ngx_str_t, service: &ngx_str_t, pool: &Pool) -> Res {
128-
let mut ctx = ResolverCtx::new(self.resolver)?;
129-
ctx.name = *name;
130-
ctx.service = *service;
131-
ctx.timeout = self.timeout;
132-
133-
Resolution::new(ctx, pool).await
127+
let mut resolver = Resolution::new(name, service, self, pool)?;
128+
resolver.as_mut().await
134129
}
135130
}
136131

@@ -149,17 +144,58 @@ struct Resolution<'a> {
149144
}
150145

151146
impl<'a> Resolution<'a> {
152-
pub fn new(mut ctx: ResolverCtx, pool: &'a Pool) -> Self {
147+
pub fn new(
148+
name: &ngx_str_t,
149+
service: &ngx_str_t,
150+
resolver: &Resolver,
151+
pool: &'a Pool,
152+
) -> Result<Pin<Box<Self, Pool>>, Error> {
153+
// Create a pinned Resolution on the Pool, so that we can make
154+
// a stable pointer to the Resolution struct.
155+
let mut this = Box::pin_in(
156+
Resolution {
157+
complete: None,
158+
waker: None,
159+
pool,
160+
ctx: None,
161+
},
162+
pool.clone(),
163+
);
164+
165+
// Set up the ctx with everything the resolver needs to resolve a
166+
// name, and the handler callback which is called on completion.
167+
let mut ctx = ResolverCtx::new(resolver.resolver)?;
168+
ctx.name = *name;
169+
ctx.service = *service;
170+
ctx.timeout = resolver.timeout;
171+
ctx.set_cancelable(1);
153172
ctx.handler = Some(Self::handler);
154173

155-
ctx.set_cancelable(1);
174+
{
175+
// Safety: Self::handler, Future::poll, and Drop::drop will have
176+
// access to &mut Resolution. Nginx is single-threaded and we are
177+
// assured only one of those is on the stack at a time, except if
178+
// Self::handler wakes a task which polls or drops the Future,
179+
// which it only does after use of &mut Resolution is complete.
180+
let ptr: &mut Resolution = unsafe { Pin::into_inner_unchecked(this.as_mut()) };
181+
ctx.data = ptr as *mut Resolution as *mut c_void;
182+
}
156183

157-
Self {
158-
complete: None,
159-
waker: None,
160-
pool,
161-
ctx: Some(ctx),
184+
// Neither ownership nor borrows are tracked for this pointer,
185+
// and ctxp will not be used after the ctx destruction.
186+
let ctxp = ctx.0;
187+
this.ctx = Some(ctx);
188+
189+
// Start name resolution using the ctx. If the name is in the dns
190+
// cache, the handler may get called from this stack. Otherwise, it
191+
// will be called later by nginx when it gets a dns response or a
192+
// timeout.
193+
let ret = unsafe { ngx_resolve_name(ctxp.as_ptr()) };
194+
if let Some(e) = NonZero::new(ret) {
195+
return Err(Error::Resolver(ResolverError::from(e), name.to_string()));
162196
}
197+
198+
Ok(this)
163199
}
164200

165201
// Nginx will call this handler when name resolution completes. If the
@@ -187,31 +223,6 @@ impl<'a> core::future::Future for Resolution<'a> {
187223
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
188224
// Resolution is Unpin, so we can use it as just a &mut Resolution
189225
let this: &mut Resolution = self.get_mut();
190-
// First time a Resolution is polled, start resolution.
191-
if this.waker.is_none() && this.complete.is_none() {
192-
// Because Resolution is pinned, the *mut pointer from &mut
193-
// Resolution can be safely used to reconstruct a &mut Resolution
194-
// in the handler anytime until the Future is Ready (which is
195-
// always preceeded by use of this addr in the handler) or
196-
// Resolution is dropped, which will drop the ResolverCtx as well,
197-
// cancelling any resolution for which the handler has not yet
198-
// been called.
199-
let addr = core::ptr::from_mut(this);
200-
201-
// Start name resolution using the ctx. If the name is in the dns
202-
// cache, the handler may get called from this stack. Otherwise, it
203-
// will be called later by nginx when it gets a dns response or a
204-
// timeout.
205-
let ctx = this
206-
.ctx
207-
.as_mut()
208-
.expect("ctx is Some between Resolution construction and handler");
209-
ctx.data = addr.cast();
210-
// Does calling the handler inside this call cause UB? We haven't
211-
// told rustc that know we have passed &mut Self to the handler by
212-
// way of the ctx.
213-
ctx.resolve()?;
214-
}
215226

216227
// The handler populates this.complete, and we consume it here:
217228
match this.complete.take() {
@@ -269,18 +280,6 @@ impl ResolverCtx {
269280
NonNull::new(ctx).map(Self).ok_or(Error::AllocationFailed)
270281
}
271282

272-
/// Starts name resolution.
273-
pub fn resolve(&mut self) -> Result<(), Error> {
274-
let ret = unsafe { ngx_resolve_name(self.0.as_ptr()) };
275-
276-
if let Some(e) = NonZero::new(ret) {
277-
let name = self.name.to_string();
278-
Err(Error::Resolver(ResolverError::from(e), name))
279-
} else {
280-
Ok(())
281-
}
282-
}
283-
284283
/// Take the results in a ctx and make an owned copy as a
285284
/// Result<Vec<ngx_addr_t, Pool>, Error>, where the Vec and the internals
286285
/// of the ngx_addr_t are allocated on the given Pool

0 commit comments

Comments
 (0)