Fix Segfault in GripperActionController #527

eholum · 2023-02-13T18:27:54Z

We have occasionally observed segfaults in the GripperActionController, particularly when pre-empting an executing goal. As far as I can tell the source is the shared goal handle pointer being reset while the action monitor loop is running.

This change wraps the raw shared_ptr for the RealtimeGoalHandlePtr in a RealtimeBuffer, and provides thread-safe access to the executing goal (if present) through the buffer. This is inline with other controller implementations, like the JointTrajectoryController.

Sample backtrace:

Thread 31 "ruby" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xffffb5fbf100 (LWP 1728392)]
0x0000ffff5c35c230 in realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::runNonRealtime (this=0x0) at /opt/ros/humble/include/realtime_tools/realtime_server_goal_handle.h:142
142        if (!valid()) {return;}
(gdb) bt
#0  0x0000ffff5c35c230 in realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::runNonRealtime (this=0x0) at /opt/ros/humble/include/realtime_tools/realtime_server_goal_handle.h:142
#1  0x0000ffff5c358ab4 in std::__invoke_impl<void, void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*&)(), std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> >&> (
    __t=std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>> (empty) = {...}, __f=@0xffffa0162c50: (void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*)(realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> * const)) 0xffff5c35c210 <realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::runNonRealtime()>) at /usr/include/c++/11/bits/invoke.h:74
#2  std::__invoke<void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*&)(), std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> >&> (
    __fn=@0xffffa0162c50: (void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*)(realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> * const)) 0xffff5c35c210 <realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::runNonRealtime()>) at /usr/include/c++/11/bits/invoke.h:96
#3  std::_Bind<void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*(std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> >))()>::__call<void, , 0ul>(std::tuple<>&&, std::_Index_tuple<0ul>) (__args=..., this=0xffffa0162c50) at /usr/include/c++/11/functional:420
#4  std::_Bind<void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*(std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> >))()>::operator()<, void>() (
    this=0xffffa0162c50) at /usr/include/c++/11/functional:503
#5  rclcpp::GenericTimer<std::_Bind<void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*(std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> >))()>, (void*)0>::execute_callback_delegate<std::_Bind<void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*(std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> >))()>, (void*)0>() (
    this=0xffffa0162c20) at /opt/ros/humble/include/rclcpp/rclcpp/timer.hpp:244
#6  rclcpp::GenericTimer<std::_Bind<void (realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand>::*(std::shared_ptr<realtime_tools::RealtimeServerGoalHandle<control_msgs::action::GripperCommand> >))()>, (void*)0>::execute_callback() (this=0xffffa0162c20) at /opt/ros/humble/include/rclcpp/rclcpp/timer.hpp:230
#7  0x0000ffffceea804c in rclcpp::Executor::execute_any_executable(rclcpp::AnyExecutable&) ()
   from /opt/ros/humble/lib/librclcpp.so
#8  0x0000ffffceea980c in rclcpp::Executor::spin_once_impl(std::chrono::duration<long, std::ratio<1l, 1000000000l> >)
    () from /opt/ros/humble/lib/librclcpp.so
#9  0x0000ffffcee9f334 in rclcpp::Executor::spin_once(std::chrono::duration<long, std::ratio<1l, 1000000000l> >) ()
   from /opt/ros/humble/lib/librclcpp.so
#10 0x0000ffffcf14b104 in operator() (__closure=0xaaaab30ab9d8) at /usr/include/c++/11/chrono:521
#11 std::__invoke_impl<void, ign_ros2_control::IgnitionROS2ControlPlugin::Configure(const Entity&, const std::shared_ptr<const sdf::v12::Element>&, ignition::gazebo::v6::EntityComponentManager&, ignition::gazebo::v6::EventManager&)::<lambda()> > (__f=...) at /usr/include/c++/11/bits/invoke.h:61
#12 std::__invoke<ign_ros2_control::IgnitionROS2ControlPlugin::Configure(const Entity&, const std::shared_ptr<const sdf::v12::Element>&, ignition::gazebo::v6::EntityComponentManager&, ignition::gazebo::v6::EventManager&)::<lambda()> > (
    __fn=...) at /usr/include/c++/11/bits/invoke.h:96
#13 std::thread::_Invoker<std::tuple<ign_ros2_control::IgnitionROS2ControlPlugin::Configure(const Entity&, const std::shared_ptr<const sdf::v12::Element>&, ignition::gazebo::v6::EntityComponentManager&, ignition::gazebo::v6::EventManager&)::<lambda()> > >::_M_invoke<0> (this=0xaaaab30ab9d8) at /usr/include/c++/11/bits/std_thread.h:253
#14 std::thread::_Invoker<std::tuple<ign_ros2_control::IgnitionROS2ControlPlugin::Configure(const Entity&, const std::shared_ptr<const sdf::v12::Element>&, ignition::gazebo::v6::EntityComponentManager&, ignition::gazebo::v6::EventManager&)::<lambda()> > >::operator() (this=0xaaaab30ab9d8) at /usr/include/c++/11/bits/std_thread.h:260
#15 std::thread::_State_impl<std::thread::_Invoker<std::tuple<ign_ros2_control::IgnitionROS2ControlPlugin::Configure(const Entity&, const std::shared_ptr<const sdf::v12::Element>&, ignition::gazebo::v6::EntityComponentManager&, ignition::gazebo::v6::EventManager&)::<lambda()> > > >::_M_run(void) (this=0xaaaab30ab9d0)
    at /usr/include/c++/11/bits/std_thread.h:211
#16 0x0000fffff3a831fc in ?? () from /lib/aarch64-linux-gnu/libstdc++.so.6
#17 0x0000fffff7aed5c8 in start_thread (arg=0x0) at ./nptl/pthread_create.c:442
#18 0x0000fffff7b55d1c in thread_start () at ../sysdeps/unix/sysv/linux/aarch64/clone.S:79

bmagyar

Good catch!
Thanks a lot for the elegant fix

codecov-commenter · 2023-02-18T06:23:28Z

Codecov Report

Merging #527 (1aeb560) into master (e7f9962) will decrease coverage by 3.31%.
The diff coverage is 26.60%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #527      +/-   ##
==========================================
- Coverage   35.78%   32.48%   -3.31%     
==========================================
  Files         189        7     -182     
  Lines       17570      665   -16905     
  Branches    11592      357   -11235     
==========================================
- Hits         6287      216    -6071     
+ Misses        994      157     -837     
+ Partials    10289      292    -9997

Flag	Coverage Δ
unittests	`32.48% <26.60%> (-3.31%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ontroller/test/test_load_diff_drive_controller.cpp	`12.50% <0.00%> (ø)`
diff_drive_controller/src/odometry.cpp	`42.16% <11.11%> (ø)`
diff_drive_controller/src/speed_limiter.cpp	`46.55% <11.11%> (ø)`
...ive_controller/test/test_diff_drive_controller.cpp	`17.62% <12.08%> (ø)`
...troller/include/diff_drive_controller/odometry.hpp	`20.00% <20.00%> (ø)`
...iff_drive_controller/src/diff_drive_controller.cpp	`39.22% <35.50%> (ø)`
...de/diff_drive_controller/diff_drive_controller.hpp	`100.00% <100.00%> (ø)`
...s/test/test_load_joint_group_effort_controller.cpp
..._controllers/src/joint_group_effort_controller.cpp
...ontrollers/src/joint_group_velocity_controller.cpp
... and 191 more

bmagyar · 2023-02-18T08:05:25Z

Could you please run pre-commit run --all to fix the format stage?

bmagyar · 2023-02-20T07:02:54Z

@Mergifyio backport to humble

mergify · 2023-02-20T07:03:08Z

backport to humble

❌ No backport have been created

Backport to branch to failed

GitHub error: Branch not found

Backport to branch humble in progress

* Use RT buffer for gripper action controller's goal handle * fix format --------- Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com> (cherry picked from commit 520036d)

bmagyar · 2023-02-20T07:04:25Z

@Mergifyio backport to humble

mergify · 2023-02-20T07:04:28Z

backport to humble

❌ No backport have been created

Backport to branch to failed

GitHub error: Branch not found

Backport to branch humble in progress

bmagyar · 2023-02-20T07:05:01Z

@Mergifyio backport humble

mergify · 2023-02-20T07:05:03Z

backport humble

❌ No backport have been created

Backport to branch humble failed

* Use RT buffer for gripper action controller's goal handle * fix format --------- Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>

* Use RT buffer for gripper action controller's goal handle * fix format --------- Co-authored-by: Erik Holum <erik.holum@picknik.ai>

Use RT buffer for gripper action controller's goal handle

1aeb560

github-actions bot requested review from anfemosa, bijoua29, bmagyar, destogl, duringhof, rosterloh and VX792 February 13, 2023 18:28

bmagyar approved these changes Feb 18, 2023

View reviewed changes

fix format

676b089

bmagyar merged commit 520036d into ros-controls:master Feb 20, 2023

bmagyar mentioned this pull request Feb 21, 2023

Fix Segfault in GripperActionController (#527) #530

Merged

bmagyar added a commit that referenced this pull request Feb 21, 2023

Fix Segfault in GripperActionController (#527) (#530)

7604cd5

* Use RT buffer for gripper action controller's goal handle * fix format --------- Co-authored-by: Erik Holum <erik.holum@picknik.ai>

destogl mentioned this pull request May 16, 2023

[Parameters] Avoid deprecation warnings." #616

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Segfault in GripperActionController #527

Fix Segfault in GripperActionController #527

eholum commented Feb 13, 2023

bmagyar left a comment

codecov-commenter commented Feb 18, 2023

bmagyar commented Feb 18, 2023

bmagyar commented Feb 20, 2023

mergify bot commented Feb 20, 2023

bmagyar commented Feb 20, 2023 •

edited

Loading

mergify bot commented Feb 20, 2023

bmagyar commented Feb 20, 2023

mergify bot commented Feb 20, 2023 •

edited

Loading

Fix Segfault in GripperActionController #527

Fix Segfault in GripperActionController #527

Conversation

eholum commented Feb 13, 2023

bmagyar left a comment

Choose a reason for hiding this comment

codecov-commenter commented Feb 18, 2023

Codecov Report

bmagyar commented Feb 18, 2023

bmagyar commented Feb 20, 2023

mergify bot commented Feb 20, 2023

❌ No backport have been created

bmagyar commented Feb 20, 2023 • edited Loading

mergify bot commented Feb 20, 2023

❌ No backport have been created

bmagyar commented Feb 20, 2023

mergify bot commented Feb 20, 2023 • edited Loading

❌ No backport have been created

bmagyar commented Feb 20, 2023 •

edited

Loading

mergify bot commented Feb 20, 2023 •

edited

Loading