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.

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

Release ownership of entities after spinning cancelled (backport #2556) #2580

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jul 10, 2024

This issue was found in ros2/rmw_fastrtps#761.
In code of MoveIt, it used static executor. When the static executor is destructed (The executor is in a cancel state.), and it's time to free up the ownership of entity resources, Fast DDS resources have already been released.

After spinning is cancelled, the Executor should release ownership of entities at once.


This is an automatic backport of pull request #2556 done by Mergify.

* Release ownership of entities after spinning cancelled

Signed-off-by: Barry Xu <[email protected]>

* Move release action to every exit point in different spin functions

Signed-off-by: Barry Xu <[email protected]>

* Move wait_result_.reset() before setting spinning to false

Signed-off-by: Barry Xu <[email protected]>

* Update test code

Signed-off-by: Barry Xu <[email protected]>

* Move test code to test_executors.cpp

Signed-off-by: Barry Xu <[email protected]>

---------

Signed-off-by: Barry Xu <[email protected]>
(cherry picked from commit 069a001)

# Conflicts:
#	rclcpp/test/rclcpp/executors/test_executors.cpp
Copy link
Contributor Author

mergify bot commented Jul 10, 2024

Cherry-pick of 069a001 has failed:

On branch mergify/bp/jazzy/pr-2556
Your branch is up to date with 'origin/jazzy'.

You are currently cherry-picking commit 069a0018.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   rclcpp/src/rclcpp/executor.cpp
	modified:   rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp
	modified:   rclcpp/src/rclcpp/executors/single_threaded_executor.cpp
	modified:   rclcpp/test/rclcpp/CMakeLists.txt

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rclcpp/test/rclcpp/executors/test_executors.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Barry-Xu-2018 i think this fix should be backported to jazzy, but git found conflict. could you resolve that?

CC: @alsora

@Barry-Xu-2018
Copy link
Collaborator

i think this fix should be backported to jazzy, but git found conflict. could you resolve that?

Okay. I will fix it.

@ahcorde
Copy link
Contributor

ahcorde commented Jul 10, 2024

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@mjcarroll
Copy link
Member

Looks like the Linux runner filled up:

10:29:37 [2738.025s] ERROR:colcon.colcon_core.event_reactor:Exception in event handler extension 'event_log': [Errno 28] No space left on device [log/build_2024-07-10_09-44-14/events.log]
10:29:52 [2738.025s] ERROR:colcon.colcon_core.event_reactor:Exception in event handler extension 'event_log': [Errno 28] No space left on device [log/build_2024-07-10_09-44-14/events.log]
10:29:52 [2738.026s] ERROR:colcon.colcon_core.event_reactor:Exception in event handler extension 'event_log': [Errno 28] No space left on device [log/build_2024-07-10_09-44-14/events.log]
10:29:52 [2738.094s] ERROR:colcon.colcon_core.event_reactor:Exception in event handler extension 'event_log': [Errno 28] No space left on device [log/build_2024-07-10_09-44-14/events.log]
10:29:52 [2738.095s] ERROR:colcon.colcon_core.event_reactor:Exception in event handler extension 'event_log': [Errno 28] No space left on device [log/build_2024-07-10_09-44-14/events.log]
10:29:52 [2738.097s] ERROR:colcon.colcon_core.event_reactor:Exception in event handler extension 'event_log': [Errno 28] No space left on device [log/build_2024-07-10_09-44-14/events.log]
10:29:52 [2738.198s] ERROR:colcon.colcon_core.event_reactor:Exception in event handler extension 'event_log': [Errno 28] No space left on device [log/build_2024-07-10_09-44-14/events.log]

@Barry-Xu-2018
Copy link
Collaborator

Re-execute

  • Linux Build Status
  • Linux-aarch64 Build Status

@Barry-Xu-2018
Copy link
Collaborator

The reason for the test failure for Linux-aarch64 is

mimick: Initialization error: could not find definitions for vital function(s): 'abort' 'vfprintf'

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Barry-Xu-2018 thanks for resolving the conflicts.

@fujitatomoya fujitatomoya merged commit d73fc2a into jazzy Jul 18, 2024
3 checks passed
@mergify mergify bot deleted the mergify/bp/jazzy/pr-2556 branch July 18, 2024 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants