Skip to content

Commit 61e3e72

Browse files
yfeldblummeta-codesync[bot]
authored andcommitted
coro async-stack: make AsyncStackFrame and AsyncStackRoot immobile
Summary: Make `AsyncStackFrame` and `AsyncStackRoot` immobile by explicitly deleting copy/move constructors and assignment operators. These structures are pointed-to by other parts of the async stack infrastructure, so moving them after they are set up would invalidate pointers and corrupt the async stack trace. Changes: - **AsyncStackFrame**: Delete copy/move ctor/assign, add `clear()` member function that resets all members to their default state (as an alternative to re-assignment). - **AsyncStackRoot**: Delete copy/move ctor/assign, add explicit default constructor. (AsyncStackRoot was already implicitly non-copyable/non-movable due to its `std::atomic` member, but making it explicit is better practice.) - **AsyncStack.cpp**: Refactor `makeDetachedRootFrame()` which returned `AsyncStackFrame` by value. Now uses a static variable with a lambda-based initializer to avoid requiring move semantics. - **AsyncGenerator.h**: The `CleanupAwaitable` class previously embedded an `AsyncStackFrame` as a direct member, but needed to be movable for the `co_withAsyncStack` customization point. Introduced a nested `Awaiter` class returned by `CleanupAwaitable::operator co_await()` that holds the `AsyncStackFrame` as a direct member. This avoids heap allocation (no `unique_ptr`/`make_unique`) while keeping `CleanupAwaitable` itself movable, since the immobile `AsyncStackFrame` now lives in the awaiter which is materialized directly in the coroutine frame via guaranteed copy elision. Also replaced `asyncFrame_ = {}` in `clearContext()` with `asyncFrame_.clear()`. All changes applied to both `fbcode/folly/` and `xplat/folly/` copies. Reviewed By: iahs Differential Revision: D95416569 fbshipit-source-id: 9c78fdf04c688193ec9db2c94acb081546f75d01
1 parent 6c56345 commit 61e3e72

5 files changed

Lines changed: 61 additions & 27 deletions

File tree

third-party/folly/src/folly/coro/AsyncGenerator.h

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -255,24 +255,34 @@ class [[nodiscard]] AsyncGenerator {
255255

256256
class [[nodiscard]] CleanupAwaitable {
257257
public:
258-
bool await_ready() noexcept { return !scopeExit_; }
259-
260-
template <typename Promise>
261-
FOLLY_NOINLINE auto await_suspend(
262-
coroutine_handle<Promise> continuation) noexcept {
263-
asyncFrame_.setReturnAddress();
264-
scopeExit_.promise().setContext(
265-
continuation, &asyncFrame_, executor_.get_alias());
266-
if constexpr (detail::promiseHasAsyncFrame_v<Promise>) {
267-
folly::pushAsyncStackFrameCallerCallee(
268-
continuation.promise().getAsyncFrame(), asyncFrame_);
269-
return scopeExit_;
270-
} else {
271-
folly::resumeCoroutineWithNewAsyncStackRoot(scopeExit_);
258+
struct Awaiter {
259+
bool await_ready() noexcept { return !scopeExit_; }
260+
261+
template <typename Promise>
262+
FOLLY_NOINLINE auto await_suspend(
263+
coroutine_handle<Promise> continuation) noexcept {
264+
asyncFrame_.setReturnAddress();
265+
scopeExit_.promise().setContext(
266+
continuation, &asyncFrame_, executor_.get_alias());
267+
if constexpr (detail::promiseHasAsyncFrame_v<Promise>) {
268+
folly::pushAsyncStackFrameCallerCallee(
269+
continuation.promise().getAsyncFrame(), asyncFrame_);
270+
return scopeExit_;
271+
} else {
272+
folly::resumeCoroutineWithNewAsyncStackRoot(scopeExit_);
273+
}
272274
}
273-
}
274275

275-
void await_resume() noexcept {}
276+
void await_resume() noexcept {}
277+
278+
coroutine_handle<detail::ScopeExitTaskPromiseBase> scopeExit_;
279+
folly::Executor::KeepAlive<> executor_;
280+
folly::AsyncStackFrame asyncFrame_;
281+
};
282+
283+
Awaiter operator co_await() && noexcept {
284+
return Awaiter{scopeExit_, std::move(executor_), {}};
285+
}
276286

277287
private:
278288
friend CleanupSemiAwaitable;
@@ -288,7 +298,6 @@ class [[nodiscard]] AsyncGenerator {
288298
}
289299

290300
coroutine_handle<detail::ScopeExitTaskPromiseBase> scopeExit_;
291-
folly::AsyncStackFrame asyncFrame_;
292301
folly::Executor::KeepAlive<> executor_;
293302
};
294303

@@ -784,7 +793,7 @@ class AsyncGeneratorPromise final
784793
executor_ = {};
785794
cancelToken_ = {};
786795
hasCancelTokenOverride_ = false;
787-
asyncFrame_ = {};
796+
asyncFrame_.clear();
788797
}
789798

790799
friend coroutine_handle<ScopeExitTaskPromiseBase> tag_invoke(

third-party/folly/src/folly/coro/CMakeLists.txt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ folly_add_library(
3434
folly_coro_auto_cleanup_fwd
3535
folly_coro_base_promise
3636
folly_coro_coroutine
37-
folly_coro_current_executor
3837
folly_coro_detail_malloc
3938
folly_coro_detail_manual_lifetime
4039
folly_coro_invoke
@@ -149,6 +148,7 @@ folly_add_library(
149148
folly_coro_via_if_async
150149
folly_coro_with_async_stack
151150
folly_executors_manual_executor
151+
folly_executors_sequenced_executor
152152
folly_fibers_core
153153
folly_lang_must_use_immediately
154154
folly_synchronization_baton
@@ -371,6 +371,7 @@ folly_add_library(
371371
HEADERS
372372
GmockHelpers.h
373373
EXPORTED_DEPS
374+
folly_coro_blocking_wait
374375
folly_coro_coroutine
375376
folly_coro_gtest_helpers
376377
folly_coro_result
@@ -455,6 +456,7 @@ folly_add_library(
455456
EXPORTED_DEPS
456457
folly_coro_via_if_async
457458
folly_exception_wrapper
459+
folly_portability
458460
)
459461

460462
folly_add_library(
@@ -625,7 +627,6 @@ folly_add_library(
625627
folly_cancellation_token
626628
folly_coro_base_promise
627629
folly_coro_coroutine
628-
folly_coro_current_executor
629630
folly_coro_detail_malloc
630631
folly_coro_detail_traits
631632
folly_coro_inline_task
@@ -742,7 +743,11 @@ folly_add_library(
742743
EXPORTED_DEPS
743744
folly_coro_task_wrapper
744745
folly_coro_via_if_async
746+
folly_lang_assume
747+
folly_result_result
748+
folly_result_try
745749
folly_result_value_only_result
750+
folly_unit
746751
)
747752

748753
folly_add_library(

third-party/folly/src/folly/tracing/AsyncStack-inl.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ inline void AsyncStackFrame::setParentFrame(AsyncStackFrame& frame) noexcept {
102102
parentFrame = &frame;
103103
}
104104

105+
inline void AsyncStackFrame::clear() noexcept {
106+
parentFrame = nullptr;
107+
instructionPointer = nullptr;
108+
stackRoot = nullptr;
109+
}
110+
105111
inline AsyncStackRoot* AsyncStackFrame::getStackRoot() noexcept {
106112
return stackRoot;
107113
}

third-party/folly/src/folly/tracing/AsyncStack.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -181,16 +181,13 @@ AsyncStackRoot& getCurrentAsyncStackRoot() noexcept {
181181
return *root;
182182
}
183183

184-
static AsyncStackFrame makeDetachedRootFrame() noexcept {
185-
AsyncStackFrame frame;
186-
frame.setReturnAddress(detached_task());
187-
return frame;
184+
AsyncStackFrame::AsyncStackFrame(MakeDetachedRootFrame) noexcept {
185+
setReturnAddress(detached_task());
188186
}
189187

190-
static AsyncStackFrame detachedRootFrame = makeDetachedRootFrame();
191-
192188
AsyncStackFrame& getDetachedRootAsyncStackFrame() noexcept {
193-
return detachedRootFrame;
189+
static auto frame = AsyncStackFrame{AsyncStackFrame::MakeDetachedRootFrame{}};
190+
return frame;
194191
}
195192

196193
#if FOLLY_HAS_COROUTINES

third-party/folly/src/folly/tracing/AsyncStack.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,13 @@ void deactivateSuspendedLeaf(AsyncStackFrame& leafFrame) noexcept;
317317
struct AsyncStackFrame {
318318
public:
319319
AsyncStackFrame() = default;
320+
~AsyncStackFrame() = default;
321+
AsyncStackFrame(const AsyncStackFrame&) = delete;
322+
AsyncStackFrame(AsyncStackFrame&&) = delete;
323+
AsyncStackFrame& operator=(const AsyncStackFrame&) = delete;
324+
AsyncStackFrame& operator=(AsyncStackFrame&&) = delete;
325+
326+
void clear() noexcept;
320327

321328
// The parent frame is the frame of the async operation that is logically
322329
// the caller of this frame.
@@ -343,6 +350,9 @@ struct AsyncStackFrame {
343350
void* getReturnAddress() const noexcept;
344351

345352
private:
353+
struct MakeDetachedRootFrame {};
354+
explicit AsyncStackFrame(MakeDetachedRootFrame) noexcept;
355+
346356
friend AsyncStackRoot;
347357

348358
friend AsyncStackFrame& getDetachedRootAsyncStackFrame() noexcept;
@@ -414,6 +424,13 @@ struct AsyncStackFrame {
414424
// list of contexts) is obtained by calling getCurrentAsyncStackRoot().
415425
struct AsyncStackRoot {
416426
public:
427+
AsyncStackRoot() = default;
428+
~AsyncStackRoot() = default;
429+
AsyncStackRoot(const AsyncStackRoot&) = delete;
430+
AsyncStackRoot(AsyncStackRoot&&) = delete;
431+
AsyncStackRoot& operator=(const AsyncStackRoot&) = delete;
432+
AsyncStackRoot& operator=(AsyncStackRoot&&) = delete;
433+
417434
// Sets the top-frame to be 'frame' and also updates the cached
418435
// 'frame.stackRoot' to be 'this'.
419436
//

0 commit comments

Comments
 (0)