Repository navigation
Resume a posted waiter only while its awaiter is alive - #63
Merged
Merged
Conversation
Channel::send/close, TfBuffer's transform arrival and send_goal's goal response post the waiting coroutine's resumption without checking that it is still waiting. A task destroyed after the post but before the executor runs it is then resumed from freed memory. Add Waiting, owned by the awaiter, and post_resume, which resumes it through a weak_ptr when the posted callback runs. A late resumption finds the weak_ptr expired with the awaiter. Add tests for the three cases, which hit a heap-use-after-free on main under ASan, and one that destroys a node-like owner of the context and its task while the task waits on an Event, a sleep or a topic.
otamachan
force-pushed
the
guard-posted-resume
branch
from
October 18, 2026 04:27
6ed73d1 to
95fdc07
Compare
This was referenced Oct 18, 2026
nishino-pfr
approved these changes
Oct 18, 2026
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
2 participants
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.
Channel::send/Channel::close,TfBuffer's transform arrival andsend_goal's goal response post the waiting coroutine's resumption with a bare handle (ctx.post([h]() { h.resume(); })). If the task is destroyed after the post but before the executor runs it, the posted callback resumes a freed frame. The awaiter destructors from #56 cannot help, because the handle is already in the queue.Add
Waiting(executor.hpp), a coroutine handle owned by the awaiter, andpost_resume, which resumes it through aweak_ptrwhen the posted callback runs. These awaiters now hand out aweak_ptr<Waiting>instead of the bare handle, so a resumption that runs late finds it expired with the awaiter. No awaiter needs a new destructor for this. Double resumption is still prevented by each awaiter's existing state (Channel'swaiter_under its mutex,TfBuffer'sactive,send_goal'sdone).TimerStreamis unchanged: its timer resumes synchronously, and its stop callback clearswaiter_on the spot.Tests:
ChannelTest.SendThenWaiterDestroyed,ChannelTest.CloseThenWaiterDestroyed,TfBufferTest.TransformThenWaiterDestroyed,TaskDestroyTest.GoalResponseThenRequesterDestroyed: each posts the resumption and then destroys the task. Built with-fsanitize=address, all four report a heap-use-after-free on main and pass with this change.TaskDestroyTest.OwnerDestroyedWhileItsTaskWaits: a node-like object that owns theCoContext, anEvent, aTopicStreamand theTaskis destroyed while the task waits on the event, a sleep or the topic, and the frame must be freed with it. This already passes on main; it guards the destroy-on-drop behaviour.colcon testpasses (352 tests, ASan build, LeakSanitizer off).This is an alternative to #60, which instead stops destroying a running frame. Under #60 the second test leaks the frame (the cancellation is posted to the context that is destroyed with the owner), and a task whose context outlives its owner unwinds into the destroyed owner.