Skip to content

fix use-after-free of the timer node mutex on destruction#1180

Open
aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:timer-node-mutex-uaf
Open

fix use-after-free of the timer node mutex on destruction#1180
aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:timer-node-mutex-uaf

Conversation

@aysha-afrah26

Copy link
Copy Markdown
Contributor

The Sleep, Delay, and Timeout nodes each own a TimerQueue whose worker thread runs the timer callback, and that callback locks a std::mutex that belongs to the node (delay_mutex_ or timeout_mutex_). Because the TimerQueue was the first-declared member it is destroyed last, after that mutex, so when a tree is torn down while a timer is still pending the worker thread (joined only in ~TimerQueue) can lock a mutex whose destructor has already run. On macOS/libc++ the pending handler then aborts the process with "mutex lock failed: Invalid argument"; on other standard libraries it is undefined behavior. It shows up as intermittent aborts in the timer and parallel teardown tests, including Parallel.Issue819_SequenceVsReactiveSequence, which the macOS CI currently skips as flaky. Declaring the TimerQueue as the last member fixes it, since ~TimerQueue() joins the worker thread while the mutex it locks is still alive. Runtime behavior for valid trees is unchanged; only the member declaration order, and therefore the destruction order, moves.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant