Skip to content

Commit 9f73a5d

Browse files
pranavtbhatmeta-codesync[bot]
authored andcommitted
Add deferred reads to the Rust channel pipeline
Summary: Add a reusable `DeferredRead` primitive that parks one type-erased inbound message together with its exact pipeline continuation. Completion and cancellation are marshalled to the originating EventBase, closed pipelines suppress late delivery, and an EventBase keepalive prevents late wakeups from targeting destroyed scheduler state. Expose `CallbackContext::defer_read` and `spawn_deferred_read` to Rust handlers without revealing the native message layout or changing the existing synchronous fast path. Reviewed By: robertroeser Differential Revision: D117026425 fbshipit-source-id: b3ff5e2c3f462eea68d2da0dadc38ecdbbada9c3
1 parent beca986 commit 9f73a5d

5 files changed

Lines changed: 230 additions & 0 deletions

File tree

thrift/lib/rust/channel_pipeline/src/context.rs

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use std::rc::Rc;
2525

2626
use crate::adapter::BytesPtr;
2727
use crate::adapter::RustMessageAdapter;
28+
use crate::erased::BorrowedMessageAdapter;
2829
use crate::event_base::EventBaseTask;
2930
use crate::event_base::FirstPoll;
3031
use crate::ffi::ffi::FfiCallbackContext;
@@ -135,6 +136,74 @@ impl Drop for ContextHandle {
135136
}
136137
}
137138

139+
/// Move-only ownership of an inbound message suspended at its pipeline
140+
/// position.
141+
///
142+
/// The original type-erased C++ message remains intact inside this token. It
143+
/// may be inspected or mutated on the originating EventBase and then resumed
144+
/// exactly once. Dropping the token cancels the read. Resume and cancellation
145+
/// are safe from any thread; native code performs delivery and destruction on
146+
/// the EventBase that owns the pipeline.
147+
pub struct DeferredRead {
148+
storage: MaybeUninit<usize>,
149+
_not_sync: PhantomData<Cell<()>>,
150+
}
151+
152+
// SAFETY: the native token has unique ownership of both the message and its
153+
// pipeline guard. Cross-thread resume and destruction consume the token and
154+
// enqueue it onto the originating EventBase before touching or destroying the
155+
// EventBase-owned values.
156+
unsafe impl Send for DeferredRead {}
157+
158+
impl DeferredRead {
159+
/// Borrow a typed view of the intact message on its originating EventBase.
160+
///
161+
/// Returns `None` off the owning EventBase. The returned borrow prevents
162+
/// this token from being resumed or dropped while the message is in use.
163+
pub fn borrow<M: BorrowedMessageAdapter>(&mut self) -> Option<M::View<'_>> {
164+
// SAFETY: this token uniquely owns one live native deferred-read token.
165+
// Native code returns its message only when called on the owning
166+
// EventBase, and the resulting borrow is tied to `&mut self`.
167+
let message =
168+
unsafe { crate::ffi::ffi::deferred_read_message(self.storage.as_mut_ptr().cast()) };
169+
if message.is_null() {
170+
return None;
171+
}
172+
// SAFETY: native code returned the address of the live message owned by
173+
// this token. `&mut self` guarantees exclusive access for the lifetime
174+
// of the pinned borrow and the token cannot move or be consumed then.
175+
let message = unsafe { &mut *message };
176+
assert!(
177+
M::holds(message),
178+
"DeferredRead::borrow: box does not hold the requested type"
179+
);
180+
// SAFETY: the unconditional `M::holds` check establishes the adapter's
181+
// exact C++ type, and the view remains tied to this exclusive borrow.
182+
Some(unsafe { M::borrow(Pin::new_unchecked(message)) })
183+
}
184+
185+
/// Resume the original inbound message from its captured pipeline
186+
/// position. Delivery is suppressed if the pipeline has closed.
187+
pub fn resume(self) {
188+
let mut deferred = std::mem::ManuallyDrop::new(self);
189+
// SAFETY: `deferred` owns one live token and ManuallyDrop prevents its
190+
// destructor from consuming that token a second time.
191+
unsafe {
192+
crate::ffi::ffi::resume_deferred_read(deferred.storage.as_mut_ptr().cast());
193+
}
194+
}
195+
}
196+
197+
impl Drop for DeferredRead {
198+
fn drop(&mut self) {
199+
// SAFETY: `DeferredRead` uniquely owns one token initialized by
200+
// `CallbackContext::defer_read`; native code consumes it exactly once.
201+
unsafe {
202+
crate::ffi::ffi::destroy_deferred_read(self.storage.as_mut_ptr().cast());
203+
}
204+
}
205+
}
206+
138207
/// Borrowed, callback-scoped view of the live C++ pipeline context.
139208
///
140209
/// Each data or lifecycle callback receives an exclusive mutable reference to a
@@ -218,6 +287,20 @@ impl<'callback> CallbackContext<'callback> {
218287
});
219288
}
220289

290+
/// Poll a future now and capture the pipeline continuation only if it suspends.
291+
///
292+
/// The future is first placed at its final pinned address and polled inline on
293+
/// the current EventBase callback. If it is ready, `ready` runs immediately
294+
/// with this borrowed context and its [`HandlerResult`] becomes the callback's
295+
/// result; no [`ContextHandle`] is created. If it is pending, the task takes a
296+
/// new one-shot `ContextHandle`, later polls remain on the same EventBase, and
297+
/// `complete` receives that handle with the output. The current callback then
298+
/// returns [`HandlerResult::Success`] because ownership of its in-flight work
299+
/// has moved into the task.
300+
///
301+
/// Panics are contained by [`EventBaseTask`]. A panic does not invoke either
302+
/// completion callback and is reported here as [`HandlerResult::Success`] so
303+
/// unwinding never crosses the C++ FFI boundary.
221304
pub(crate) fn spawn_deferred<T, Fut, Ready, Complete>(
222305
&mut self,
223306
future: Fut,
@@ -257,6 +340,67 @@ impl<'callback> CallbackContext<'callback> {
257340
handle
258341
}
259342

343+
/// Suspend the current inbound message without unpacking or copying it.
344+
///
345+
/// Returns `None` if this callback has no message, the message is empty, or
346+
/// it was already forwarded. Dropping the returned token cancels delivery;
347+
/// [`DeferredRead::resume`] continues it from this handler's exact position.
348+
pub fn defer_read(
349+
&mut self,
350+
message: crate::erased::RustTypeErasedBox<'_>,
351+
) -> Option<DeferredRead> {
352+
let _ = &message;
353+
let mut deferred = DeferredRead {
354+
storage: MaybeUninit::uninit(),
355+
_not_sync: PhantomData,
356+
};
357+
// SAFETY: storage is one pointer-aligned word and remains
358+
// exclusively owned by `deferred`. On false, native code constructed
359+
// nothing, so mem::forget prevents running a destructor on garbage.
360+
let initialized = unsafe {
361+
self.inner
362+
.as_mut()
363+
.init_deferred_read(deferred.storage.as_mut_ptr().cast())
364+
};
365+
if initialized {
366+
Some(deferred)
367+
} else {
368+
std::mem::forget(deferred);
369+
None
370+
}
371+
}
372+
373+
/// Suspend the current inbound message while a future runs on this
374+
/// pipeline's EventBase.
375+
///
376+
/// Completion receives the intact message token and may borrow, mutate,
377+
/// resume, or cancel it. The task owns the token throughout suspension, so
378+
/// panic or task destruction safely cancels the read.
379+
///
380+
/// If the current callback has no live message, the message is empty, or it
381+
/// was already forwarded, this returns HandlerResult::Error without
382+
/// polling future or invoking complete.
383+
pub fn spawn_deferred_read<T, Fut, Complete>(
384+
&mut self,
385+
message: crate::erased::RustTypeErasedBox<'_>,
386+
future: Fut,
387+
complete: Complete,
388+
) -> HandlerResult
389+
where
390+
T: Send + 'static,
391+
Fut: Future<Output = T> + Send + 'static,
392+
Complete: FnOnce(DeferredRead, T) + Send + 'static,
393+
{
394+
let Some(deferred) = self.defer_read(message) else {
395+
return HandlerResult::Error;
396+
};
397+
let event_base = self.inner.as_ref().get_ref().event_base();
398+
EventBaseTask::start(event_base, async move {
399+
complete(deferred, future.await);
400+
});
401+
HandlerResult::Success
402+
}
403+
260404
/// Forward the inbound buffer downstream and return the result.
261405
///
262406
/// The buffer is moved into the C++ message box and forwarded via

thrift/lib/rust/channel_pipeline/src/ffi.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ pub(crate) mod ffi {
136136
#[cxx_name = "initContextHandle"]
137137
unsafe fn init_context_handle(self: Pin<&mut FfiCallbackContext>, storage: *mut u8);
138138

139+
/// SAFETY: `storage` must point to one pointer-sized, pointer-aligned,
140+
/// uninitialized word. On success it contains one live token until
141+
/// resumed or destroyed.
142+
#[cxx_name = "initDeferredRead"]
143+
unsafe fn init_deferred_read(self: Pin<&mut FfiCallbackContext>, storage: *mut u8) -> bool;
144+
139145
#[namespace = "folly"]
140146
type EventBase;
141147
#[cxx_name = "eventBase"]
@@ -222,6 +228,19 @@ pub(crate) mod ffi {
222228
/// live guard back to the originating EventBase when necessary.
223229
#[cxx_name = "destroyContextHandle"]
224230
unsafe fn destroy_context_handle(storage: *mut u8);
231+
232+
/// SAFETY: `storage` must contain one live token constructed by
233+
/// `init_deferred_read`. Both functions consume it exactly once.
234+
#[cxx_name = "resumeDeferredRead"]
235+
unsafe fn resume_deferred_read(storage: *mut u8);
236+
/// SAFETY: same token contract as `resume_deferred_read`.
237+
#[cxx_name = "destroyDeferredRead"]
238+
unsafe fn destroy_deferred_read(storage: *mut u8);
239+
/// SAFETY: `storage` must contain a live deferred-read token. The
240+
/// returned pointer is non-null only on its originating EventBase and
241+
/// remains borrowed from that token.
242+
#[cxx_name = "deferredReadMessage"]
243+
unsafe fn deferred_read_message(storage: *mut u8) -> *mut TypeErasedBox;
225244
}
226245

227246
#[namespace = "folly"]

thrift/lib/rust/channel_pipeline/src/handler.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,25 @@ impl RustHandler for ContextHandleTestHandler {
360360
});
361361
return handler.fire_read(ctx, msg);
362362
}
363+
27 | 28 => {
364+
let (wake, woke) = WorkerWakeFuture::new();
365+
let result = ctx.spawn_deferred_read(msg, wake, |deferred, ()| {
366+
deferred.resume();
367+
});
368+
woke.recv().expect("worker should issue the EventBase wake");
369+
if self.scenario == 28 {
370+
ctx.close();
371+
}
372+
return result;
373+
}
374+
29 => {
375+
let deferred = ctx
376+
.defer_read(msg)
377+
.expect("the callback should own one live inbound message");
378+
std::thread::spawn(move || drop(deferred))
379+
.join()
380+
.expect("off-thread DeferredRead cancellation should not panic");
381+
}
363382
24 => {
364383
drop(msg.take::<BytesPtr>());
365384
let mut handler = CoroExceptionHandle::new(|error| async move { error });

thrift/lib/rust/channel_pipeline/src/integration_test.rs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use std::sync::atomic::Ordering;
2020
use channel_pipeline::BytesPtr;
2121
use channel_pipeline::CallbackContext;
2222
use channel_pipeline::ContextHandle;
23+
use channel_pipeline::DeferredRead;
2324
use channel_pipeline::HandlerResult;
2425
use channel_pipeline::RustHandler;
2526
use channel_pipeline::RustHandlerOpaque;
@@ -720,6 +721,18 @@ fn context_handle_is_send_not_sync_clone_or_copy() {
720721
});
721722
}
722723

724+
#[test]
725+
fn deferred_read_is_send_not_sync_clone_or_copy() {
726+
run_with_timeout(|| {
727+
static_assertions::assert_impl_all!(DeferredRead: Send);
728+
static_assertions::assert_not_impl_any!(DeferredRead: Sync, Clone, Copy);
729+
assert_eq!(
730+
std::mem::size_of::<DeferredRead>(),
731+
std::mem::size_of::<usize>()
732+
);
733+
});
734+
}
735+
723736
#[test]
724737
fn native_context_handle_token_traits_match_rust_abi() {
725738
run_with_timeout(|| {
@@ -896,6 +909,35 @@ fn coro_read_worker_wake_repolls_and_resumes_read() {
896909
assert_context_handle_read_sandwich(&ffi::run_context_handle_sandwich_test(21), false);
897910
}
898911

912+
#[test]
913+
fn deferred_read_worker_wake_resumes_original_erased_message() {
914+
assert_context_handle_read_sandwich(&ffi::run_context_handle_sandwich_test(27), false);
915+
}
916+
917+
#[test]
918+
fn deferred_read_resume_after_close_is_suppressed() {
919+
let result = ffi::run_context_handle_sandwich_test(28);
920+
assert_eq!(result.before_reads_before_fence, 1);
921+
assert_eq!(result.before_reads_after_fence, 1);
922+
assert_eq!(result.after_reads_before_fence, 0);
923+
assert_eq!(result.after_reads_after_fence, 0);
924+
assert_eq!(result.endpoint_calls_before_fence, 0);
925+
assert_eq!(result.endpoint_calls_after_fence, 0);
926+
assert!(!result.pointer_identity_preserved);
927+
}
928+
929+
#[test]
930+
fn deferred_read_off_thread_drop_cancels_safely() {
931+
let result = ffi::run_context_handle_sandwich_test(29);
932+
assert_eq!(result.before_reads_before_fence, 1);
933+
assert_eq!(result.before_reads_after_fence, 1);
934+
assert_eq!(result.after_reads_before_fence, 0);
935+
assert_eq!(result.after_reads_after_fence, 0);
936+
assert_eq!(result.endpoint_calls_before_fence, 0);
937+
assert_eq!(result.endpoint_calls_after_fence, 0);
938+
assert!(!result.pointer_identity_preserved);
939+
}
940+
899941
#[test]
900942
fn coro_write_ready_future_polls_inline_and_resumes_write() {
901943
assert_context_handle_write_sandwich(&ffi::run_context_handle_sandwich_test(22), true);

thrift/lib/rust/channel_pipeline/src/lib.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@
5353
//! later wakes schedule further polls back onto that same EventBase. The task
5454
//! owns a [`ContextHandle`], which retains the pipeline until the task
5555
//! completes or is cancelled.
56+
//! - [`CallbackContext::defer_read`] moves the intact inbound erased message
57+
//! and its continuation into a one-word [`DeferredRead`] token. Typed opaque
58+
//! views can be borrowed on the owning EventBase, and consuming `resume`
59+
//! forwards the original box without copying the message payload.
5660
//! - [`CoroReadHandle`], [`CoroWriteHandle`], and [`CoroExceptionHandle`] wrap
5761
//! an `async` handler body so the message or error round-trips through a
5862
//! future and resumes the pipeline on completion.
@@ -143,6 +147,7 @@
143147
//! | [`box_handler`] | Type-erases a handler for a downstream CXX factory |
144148
//! | [`CallbackContext`] | Borrowed pipeline context — `!Send`, `!Sync`, non-escapable |
145149
//! | [`ContextHandle`] | Move-only captured pipeline position; consuming `fire_read`/`fire_write`/`fire_exception` |
150+
//! | [`DeferredRead`] | Move-only suspended inbound message plus its exact pipeline continuation |
146151
//! | [`RustTypeErasedBox`] | Borrowed, type-erased message box; recover the value with `take::<T>()` |
147152
//! | [`BytesPtr`] | Zero-copy `unique_ptr<folly::IOBuf>` adapter |
148153
//! | [`PipelineError`] | Owned Rust error converted to `folly::exception_wrapper` |
@@ -256,6 +261,7 @@ pub use adapter::BytesPtr;
256261
pub use adapter::RustMessageAdapter;
257262
pub use context::CallbackContext;
258263
pub use context::ContextHandle;
264+
pub use context::DeferredRead;
259265
pub use context::PipelineError;
260266
pub use coro_handler::ContextReadMessage;
261267
pub use coro_handler::ContextWriteMessage;

0 commit comments

Comments
 (0)