Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MultiThreadedAgnocastExecutor may crash by assert in get_next_ready_executable_from_map() #469

Open
atsushi421 opened this issue Mar 4, 2025 · 1 comment · May be fixed by #481
Open

MultiThreadedAgnocastExecutor may crash by assert in get_next_ready_executable_from_map() #469

atsushi421 opened this issue Mar 4, 2025 · 1 comment · May be fixed by #481
Assignees
@sykwer @atsushi421
Labels
bug Something isn't working must

Comments

@atsushi421
Copy link
Contributor

atsushi421 commented Mar 4, 2025 •
edited
Loading

MultiThreadedAgnocastExecutor::ros2_spin() は Executor::get_next_executable() -> Executor::get_next_ready_executable() -> Executor::get_next_ready_executable_from_map() を呼び出す。get_next_ready_executable_from_map() は success=true の際に、この行で can_be_taken_from()がtrueであることをassertするが、success=true ~895行目の間に execute_agnocast_executables() によって can_be_taken_from() が false に更新されると、このassert文でabortしてしまう。

仮にassertが無かったとしても、MutuallyExclusiveのルールが破られてしまう (agnocast callbackとros2 callbackが同時に実行されうる) ので、いずれにせよバグである。

Related Slack
@atsushi421 atsushi421 added bug Something isn't working must labels Mar 4, 2025
@atsushi421 atsushi421 self-assigned this Mar 4, 2025
@sykwer sykwer self-assigned this Mar 4, 2025
@atsushi421 atsushi421 changed the title rclcpp::MultiThreadedExecutor may crash by assert in get_next_ready_executable_from_map() MultiThreadedAgnocastExecutor may crash by assert in get_next_ready_executable_from_map() Mar 4, 2025
@atsushi421
Copy link
Contributor Author

agnocast側でも std::lock_guard wait_lock{wait_mutex_}; を取れば解決できそう
https://github.com/ros2/rclcpp/blob/19773973a80d753c6fa028b0b548462fbbef122d/rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp#L81

@atsushi421 atsushi421 linked a pull request Mar 6, 2025 that will close this issue
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sykwer sykwer

@atsushi421 atsushi421

Labels
bug Something isn't working must
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(MultiThreadedAgnocastExecutor): add mutex to preserve callback group rule tier4/agnocast
2 participants
@sykwer @atsushi421