Repository navigation
Conversation
otamachan
force-pushed
the
detach-running-task
branch
from
October 6, 2026 13:55
5ac413e to
5d65224
Compare
otamachan
force-pushed
the
detach-running-task
branch
from
October 18, 2026 03:41
5d65224 to
5949b6f
Compare
~Task() destroyed the coroutine frame even while it was suspended in an awaiter. Everything that awaiter had handed the handle to -- a stream's waiter, an Event queue, a pending service response, a posted resumption, a TF pending request -- then pointed at freed memory, and each awaiter grew its own guard against that (is_done predicates, weak StopCb checks, the destructors from #56). A running frame is now never destroyed by its owner. ~Task() cancels it and lets go: the awaiter resumes it on the executor, CancelledException unwinds the body, and FinalAwaiter frees the frame. With a live frame behind every handed out handle, the per-awaiter destructors are no longer needed and are removed. create_task now posts the start instead of resuming on the spot, so a task starts after whatever was scheduled before it -- the unwinding of a task cancelled just before, say -- and a task released before its start never runs. This relies on the callable passed to create_task being kept alive (previous commit). Add tests for the ownership rule, the start order, and the remaining cases where a resumption was posted before the task was destroyed (Channel, TfBuffer, send_goal), which crashed on main.
otamachan
force-pushed
the
detach-running-task
branch
from
October 18, 2026 03:49
5949b6f to
2c8a233
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
~Task()destroyed the coroutine frame even while it was suspended in an awaiter. Everything that awaiter had handed the handle to -- a stream'swaiter_, anEventqueue, a pending service response, a posted resumption, a TF pending request -- then pointed at freed memory. Each awaiter grew its own guard against that (is_donepredicates, weakStopCbchecks, the destructors from #56), and every new awaiter would need one more.This changes the ownership rule instead: a running frame is never destroyed by its owner.
~Task()(and move-assignment over a running task) requests stop and lets go of the frame. The awaiter resumes it on the executor as it already does forcancel(),CancelledExceptionunwinds the body, andFinalAwaiterfrees the frame. With a live frame behind every handed-out handle, the per-awaiter destructors from #56 are no longer needed and are removed.create_tasknow posts the start instead of resuming on the spot. A task therefore starts after whatever was scheduled before it (e.g. the unwinding of a task cancelled just before), and a task released before its start never runs. Since the body no longer runs insidecreate_task, a lambda passed to it must outlive the call: this PR includes the commit from #59, which moves the callable into the task's frame. Merge #59 first and this PR shrinks to one commit, or merge this and close #59.Visible changes, documented in the guide:
catch (const CancelledException &)handlers run after the owner has moved on.shielded task) lives until that awaitable completes, instead of being freed under it.create_taskreturns before the coroutine has run.task = ctx.create_task(...)evaluates the right-hand side first, so to have the old task unwind before the new one starts, calltask.cancel()first.Tests:
DestroyedTaskUnwindsOnTheExecutor,DestroyedTaskWithoutCancellableAwaiterOutlivesItsOwner,TaskDestroyedBeforeItStartsNeverRunsandReplacedTaskUnwindsBeforeItsReplacementStartspin down the new rules.ChannelTest.SendThenWaiterDestroyed/CloseThenWaiterDestroyed,TfBufferTest.TransformThenWaiterDestroyedandTaskDestroyTest.GoalResponseThenRequesterDestroyeddestroy the task after its resumption was posted but before it ran. They crash on main (segfault in Debug, heap-use-after-free under ASan) and pass here with no awaiter-side change; they supersede Skip a posted resumption whose awaiter is gone #58.Full suite passes in Debug (396 tests, lint included). Under ASan every test binary is clean.