fix(cie): fix cancel_executor race condition#1278
Open
atsushi421 wants to merge 1 commit intomainfrom
Open
Conversation
The `thread_initialized` flag was set before `spin()` internally sets `spinning=true`. If `cancel_executor` observed `thread_initialized==true` and skipped the `is_spinning()` wait, it could call `cancel()` before `spin()` set `spinning=true`, causing `spin()` to run indefinitely and block `join()` forever. Remove the `thread_initialized` flag entirely and always wait for `is_spinning()` before calling `cancel()`. Also add a `joinable()` guard to avoid UB if the thread was never started. Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
atsushi421
added
the
need-patch-update
Contributor
There was a problem hiding this comment.
Pull request overview
Ports an upstream fix to prevent a race in cancel_executor() where cancellation could be lost if it happened before the executor’s spin() marks itself as spinning, potentially causing join() to block indefinitely.
Changes:
- Remove the
thread_initialized_fast-path and always wait forexecutor_->is_spinning()before callingcancel(). - Remove
thread_initialized_fromExecutorWrapper. - Add a
thread_.joinable()guard to avoid undefined behavior when joining.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kobayu858
approved these changes
Apr 23, 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
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.
Description
Port race condition fix from upstream callback_isolated_executor#52.
The
thread_initialized_flag was set totruebeforespin()internally setsspinning=true. Ifcancel_executorobservedthread_initialized_==trueand skipped theis_spinning()wait, it could callcancel()(which setsspinning=false) beforespin()setspinning=true. The subsequentspin()would override the cancellation and run indefinitely, blockingthread.join()forever.The fix:
thread_initialized_fast-path and always wait foris_spinning()before callingcancel().thread_initialized_field fromExecutorWrapperentirely.joinable()guard to avoid undefined behavior if the thread was never started.Related links
How was this PR tested?
bash scripts/test/e2e_test_1to1.bash(required)bash scripts/test/e2e_test_2to2.bash(required)bash scripts/test/run_requires_kernel_module_tests.bash(required)Notes for reviewers
Version Update Label (Required)
Please add exactly one of the following labels to this PR:
need-major-update: User API breaking changesneed-minor-update: Internal API breaking changes (heaphook/kmod/agnocastlib compatibility)need-patch-update: Bug fixes and other changesImportant notes:
need-major-updateorneed-minor-update, please include this in the PR title as well.fix(foo)[needs major version update]: barorfeat(baz)[needs minor version update]: quxrun-build-testlabel. The PR can only be merged after the build tests pass.See CONTRIBUTING.md for detailed versioning rules.