Skip to content

[humble] Gracefully handle SIGINT and SIGTERM in rosbag2 recorder (backport #1301)#1395

Merged
MichaelOrlov merged 3 commits intohumblefrom
mergify/bp/humble/pr-1301
Jul 4, 2023
Merged

[humble] Gracefully handle SIGINT and SIGTERM in rosbag2 recorder (backport #1301)#1395
MichaelOrlov merged 3 commits intohumblefrom
mergify/bp/humble/pr-1301

Conversation

@mergify
Copy link

@mergify mergify bot commented Jun 13, 2023 •
edited by MichaelOrlov
Loading

This is an automatic backport of pull request #1301 done by Mergify.
Cherry-pick of 46a23e9 has failed:

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

You are currently cherry-picking commit 46a23e9.
  (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)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   rosbag2_py/src/rosbag2_py/_transport.cpp
	both modified:   rosbag2_py/test/test_transport.py

no changes added to commit (use "git add" and/or "git commit -a")

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

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@mergify mergify bot requested a review from a team as a code owner June 13, 2023 06:24
@mergify mergify bot requested review from MichaelOrlov and jhdcs and removed request for a team June 13, 2023 06:24
@mergify mergify bot added the conflicts label Jun 13, 2023
@mergify mergify bot mentioned this pull request Jun 13, 2023
Merged
@MichaelOrlov MichaelOrlov added the humble Humble Hawksbill label Jun 13, 2023
@MichaelOrlov MichaelOrlov changed the title Gracefully handle SIGINT and SIGTERM in rosbag2 recorder (backport #1301) [humble] Gracefully handle SIGINT and SIGTERM in rosbag2 recorder (backport #1301) Jun 13, 2023
@MichaelOrlov MichaelOrlov force-pushed the mergify/bp/humble/pr-1301 branch 2 times, most recently from 29bd1d7 to 029d39a Compare June 13, 2023 19:21
@MichaelOrlov MichaelOrlov force-pushed the mergify/bp/humble/pr-1301 branch from 029d39a to 208b862 Compare June 13, 2023 23:26
@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jun 13, 2023

Gist: https://gist.githubusercontent.com/MichaelOrlov/1835be9737b4641f41a11e759c921138/raw/b76a9e9cecdfbfc0812ca3be7625b1bf96eb5b67/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_py rosbag2_tests
TEST args: --packages-above rosbag2_py rosbag2_tests
ROS Distro: humble
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12235

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

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jun 27, 2023

Re-run CI after rebase.
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12302

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

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jun 27, 2023

Ups it was a wrong PR for re-running CI - canceled.

@morlov-apexai
Gracefully handle SIGINT and SIGTERM in rosbag2 recorder (#1301)
6fbfb84 
* Gracefully handle SIGINT and SIGTERM signal for rosbag2 recorder

- Call recorder->stop() and exec_->cancel() instead of rclcpp::shutdown

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Use singleton for static executor in the rosbag2_py::Recorder

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Rollback to the non-static executor and don't call executor->cancel()

- In case when signal will arrive we will trigger our internal exit_
variable and wait while current exec_->spin_all(10msec) will exit.

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Spin recorder node in a separate thread for better handling exit

- Run exec->spin() in a separate thread, because we need to call
exec->cancel() after recorder->stop() to be able to send notifications
about bag split and close.
- Wait on conditional variable for exit_ flag

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Address race condition in rosbag2_py.test_record_cancel

- Add `record_thread.join()` before trying to parse metadata.

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

---------

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
(cherry picked from commit 46a23e9)

# Conflicts:
#	rosbag2_py/src/rosbag2_py/_transport.cpp
#	rosbag2_py/test/test_transport.py
@morlov-apexai
Address merge conflicts
8a8c8c0 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@morlov-apexai
Workaround for segmentation fault in rclcpp::SignalHandler::uninstall()
3e891a3 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov force-pushed the mergify/bp/humble/pr-1301 branch from 208b862 to 3e891a3 Compare July 3, 2023 18:39
@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jul 3, 2023

Re-run CI after workaround segmentation fault in rclcpp::SignalHandler.
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12314

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

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jul 3, 2023

Ups occasionally ran CI for the wrong branch.
Re-run it
Gist: https://gist.githubusercontent.com/MichaelOrlov/1835be9737b4641f41a11e759c921138/raw/b76a9e9cecdfbfc0812ca3be7625b1bf96eb5b67/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_py rosbag2_tests
TEST args: --packages-above rosbag2_py rosbag2_tests
ROS Distro: humble
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/12315

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

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jul 4, 2023

Linux-aarch64 build failed due to CI infrastructure issues.
Re-run it one more time

  • Linux-aarch64 Build Status

@MichaelOrlov MichaelOrlov merged commit e97cf9e into humble Jul 4, 2023
@delete-merged-branch delete-merged-branch bot deleted the mergify/bp/humble/pr-1301 branch July 4, 2023 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emersonknapp emersonknapp emersonknapp approved these changes

@MichaelOrlov MichaelOrlov MichaelOrlov approved these changes

@jhdcs jhdcs Awaiting requested review from jhdcs jhdcs was automatically assigned from ros2/tooling-reviewers

Assignees

No one assigned

Labels

humble Humble Hawksbill

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MichaelOrlov @emersonknapp @morlov-apexai