Skip to content

Commit 4613377

Browse files
Fix double-free on GCC-12 in CoroTaskRunner frame destruction
Replace `task_ = {}` with `std::move(task_)` in resume() and expectEarlyExit(). The move assignment operator calls handle_.destroy() while task_.handle_ still holds the old (now dangling) handle value. If frame destruction triggers re-entrant runner cleanup on GCC-12, the destructor sees a non-null handle_ and destroys the same frame again — a double-free. std::move(task_) immediately nulls task_.handle_ via the move constructor, then the frame is destroyed when the local goes out of scope. This eliminates the re-entrancy window. Also remove storedFunc_.reset() from resume() — the callable does not participate in the shared_ptr cycle and will be cleaned up by the runner's destructor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 51ae47c commit 4613377

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

include/xrpl/core/CoroTaskRunner.ipp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,12 @@ JobQueue::CoroTaskRunner::resume()
256256
#ifndef NDEBUG
257257
finished_ = true;
258258
#endif
259-
// Destroy the coroutine frame to break the shared_ptr cycle:
260-
// frame -> lambda captures shared_ptr<CoroTaskRunner> -> this.
261-
// Also release the heap-stored callable (no longer needed).
262-
task_ = {};
263-
storedFunc_.reset();
259+
// Break the shared_ptr cycle: frame -> shared_ptr<runner> -> this.
260+
// Use std::move (not task_ = {}) so task_.handle_ is null BEFORE the
261+
// frame is destroyed. operator= would destroy the frame while handle_
262+
// still holds the old value -- a re-entrancy hazard on GCC-12 if
263+
// frame destruction triggers runner cleanup.
264+
[[maybe_unused]] auto completed = std::move(task_);
264265
}
265266
std::lock_guard lk(mutex_run_);
266267
running_ = false;
@@ -296,11 +297,13 @@ JobQueue::CoroTaskRunner::expectEarlyExit()
296297
finished_ = true;
297298
#endif
298299
}
299-
// Destroy the coroutine frame to break a potential shared_ptr cycle.
300+
// Break the shared_ptr cycle: frame -> shared_ptr<runner> -> this.
300301
// The coroutine is at initial_suspend and never ran user code, so
301-
// destroying it is safe. Without this, the frame holds a shared_ptr
302-
// back to this CoroTaskRunner, creating an unreachable reference cycle.
303-
task_ = {};
302+
// destroying it is safe. Use std::move (not task_ = {}) so
303+
// task_.handle_ is null before the frame is destroyed.
304+
{
305+
[[maybe_unused]] auto completed = std::move(task_);
306+
}
304307
storedFunc_.reset();
305308
}
306309

0 commit comments

Comments
 (0)