Repository navigation
fix stale aborted timer handler ending a restarted SleepNode early - #1267
Open
aysha-afrah26 wants to merge 1 commit into
Open
aysha-afrah26 wants to merge 1 commit into
aysha-afrah26 wants to merge 1 commit into
Conversation
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.
When a Sleep node is halted, onHalted() cancels its timer through TimerQueue::cancel(), which does not wait: it re-queues the handler so the worker thread runs it shortly after with aborted=true. That handler cleared timer_waiting_ unconditionally, even though onHalted() had already done it. If the Sleep is started again before the worker gets to the canceled handler, which is what happens with tree.haltTree() followed by a new tick or with a parent that halts and re-ticks it, onStart() sets timer_waiting_ for the new timer and the stale handler then resets it, so the next onRunning() returns SUCCESS. In practice a finished after about 20 ms in 44 of 50 halt/restart cycles on macOS, which matters for trees that rely on Sleep to wait before acting. The aborted path now returns without touching the node, and only a timer that really expired clears the flag and emits the wake-up signal. The change stays inside sleep_node.cpp with no header, layout, API or ABI change, and Decorator.SleepRestartedAfterHaltWaitsFullDuration fails on master at the first iteration and passes with the fix; the full ctest suite is green locally apart from cmake_subproject_version, which fails the same way on the untouched tree because of my local SDK, and clang-format is clean (clang-tidy-21 was not available here).