Skip to content

[moveit_ros] fix race condition when stopping trajectory execution (backport #3198) - #3240

Merged
sea-bass merged 1 commit into
jazzyfrom
mergify/bp/jazzy/pr-3198
Jan 12, 2025
Merged

sea-bass merged 1 commit into
jazzyfrom
mergify/bp/jazzy/pr-3198

Conversation

@mergify

@mergify mergify Bot commented Jan 12, 2025

Copy link
Copy Markdown

Description

It is not thread-safe calling stopExecution or publishing a "stop" to trajectory_execution_event while trajectory executing. Sometimes it crashes, here's a sample

Program terminated with signal SIGSEGV, Segmentation fault.

#0 0x0000ffffa367ac48 in _gnu_cxx::normal_iterator<moveit_msgs::msg::RobotTrajectory<std::allocator > const*, std::vector<moveit_msgs::msg::RobotTrajectory<std::allocator >, std::allocator<moveit_msgs::msg::RobotTrajectory<std::allocator > > > >::__normal_iterator (__i=, this=)
at /usr/include/c++/11/bits/stl_iterator.h:1028
b1028 : _M_current(__i) { }
[Current thread is 1 (Thread 0xffff1e7cf120 (LWP 4402))]
(gdb)
(gdb)
(gdb) bt
#0 0x0000ffffa367ac48 in _gnu_cxx::normal_iterator<moveit_msgs::msg::RobotTrajectory<std::allocator > const*, std::vector<moveit_msgs::msg::RobotTrajectory<std::allocator >, std::allocator<moveit_msgs::msg::RobotTrajectory<std::allocator > > > >::__normal_iterator (i=, this=)
at /usr/include/c++/11/bits/stl_iterator.h:1028
#1 std::vector<moveit_msgs::msg::RobotTrajectory
<std::allocator >, std::allocator<moveit_msgs::msg::RobotTrajectory
<std::allocator > > >::end (
this=) at /usr/include/c++/11/bits/stl_vector.h:839
#2 trajectory_execution_manager::TrajectoryExecutionManager::waitForRobotToStop (this=this@entry=0xaaaaeed29f20, context=..., wait_time=wait_time@entry=1)
at /workspace/source/planning/trajectory_execution_manager/src/trajectory_execution_manager.cpp:1592
#3 0x0000ffffa36829d4 in trajectory_execution_manager::TrajectoryExecutionManager::executeThread(std::function<void (moveit_controller_manager::ExecutionStatus const&)> const&, std::function<void (unsigned long)> const&, bool) (this=0xaaaaeed29f20, callback=..., part_callback=..., auto_clear=) at /usr/include/c++/11/bits/stl_vector.h:1043
#4 0x0000ffffba7431fc in ?? () from /lib/aarch64-linux-gnu/libstdc++.so.6
#5 0x0000ffffbb92d5c8 in start_thread (arg=0x0) at ./nptl/pthread_create.c:442
#6 0x0000ffffbb995edc in thread_start () at ../sysdeps/unix/sysv/linux/aarch64/clone.S:79

This CL introduces a lock for interactions with trajectories_ and can fix this issue.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

This is an automatic backport of pull request #3198 done by [Mergify](https://mergify.com).

…3198)

Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
(cherry picked from commit 6595dee)
@codecov-commenter

codecov-commenter commented Jan 12, 2025 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 44.18%. Comparing base (dfbf1d9) to head (9b11111).
Report is 3 commits behind head on jazzy.

Files with missing lines Patch % Lines
...ution_manager/src/trajectory_execution_manager.cpp 0.00% 6 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##            jazzy    #3240      +/-   ##
==========================================
- Coverage   44.18%   44.18%   -0.00%     
==========================================
  Files         698      698              
  Lines       61588    61593       +5     
  Branches     7459     7460       +1     
==========================================
  Hits        27207    27207              
- Misses      34213    34218       +5     
  Partials      168      168              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sea-bass
sea-bass merged commit 7456556 into jazzy Jan 12, 2025
@sea-bass
sea-bass deleted the mergify/bp/jazzy/pr-3198 branch January 12, 2025 20:30
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.

3 participants