Skip to content

fix(rclcpp_action): Fix sleep of expire thread in case of canceled timer #2800

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
Merged
merged 1 commit into from
Apr 11, 2025
Merged

fix(rclcpp_action): Fix sleep of expire thread in case of canceled timer #2800

merged 1 commit into from
Apr 11, 2025

Conversation

jmachowinski
Copy link
Collaborator

This fixes a bug, that the expire action thread would not sleep as, the sleep duration was not computed correctly.

Fixes #2799

@jmachowinski jmachowinski mentioned this pull request Apr 10, 2025
Closed
@jmachowinski
Copy link
Collaborator Author

Pulls: #2800
Gist: https://gist.githubusercontent.com/jmachowinski/b788276df668933ea120184f152756af/raw/79f8aa943bf792cecb497d0c0234b9a887e5bf27/ros2.repos
BUILD args:
TEST args:
ROS Distro: jazzy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15652

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

fix(rclcpp_action): Fix sleep of expire thread in case of canceled timer
740e219 
This fixes a bug, that the expire action thread would not sleep as,
the sleep duration was not computed correctly.

Signed-off-by: Janosch Machowinski <[email protected]>
@jmachowinski
Copy link
Collaborator Author

Pulls: #2800
Gist: https://gist.githubusercontent.com/jmachowinski/3eb0cc44967b6a96ae007084db7be5b1/raw/79f8aa943bf792cecb497d0c0234b9a887e5bf27/ros2.repos
BUILD args:
TEST args:
ROS Distro: jazzy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15659

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

@Timple
Copy link
Contributor

Timple commented Apr 11, 2025

@marcoag if this fix is merged, would a quick re-sync for Jazzy be possible?

@jmachowinski
Copy link
Collaborator Author

Test failures are unrelated.
test_clock : multiple_threads_wait_on_one_clock is missing mutexes. We should make a good first issue ticket from this one...

@jmachowinski jmachowinski merged commit f3d66b8 into ros2:jazzy Apr 11, 2025
3 checks passed
@jmachowinski
Copy link
Collaborator Author

FYI #2804

@marcoag
Copy link
Contributor

marcoag commented Apr 15, 2025 •
edited
Loading

@marcoag if this fix is merged, would a quick re-sync for Jazzy be possible?

Will try to get one out asap, so we can put the fix out, thanks @Timple!

@Timple
Copy link
Contributor

Timple commented Apr 22, 2025

Cool, what would asap look like? As a typical cadence is 3 weeks and we're past week 2 😉

@marcoag
Copy link
Contributor

marcoag commented Apr 22, 2025

Cool, what would asap look like? As a typical cadence is 3 weeks and we're past week 2 😉

I'll put the hold today and get everything ready. I was waiting for a fix on the current regression but there doesn't seem much movement there, I'll give them one more day before I sync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MCFurry MCFurry MCFurry left review comments

@christophebedard christophebedard christophebedard approved these changes

@Timple Timple Timple approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jmachowinski @Timple @marcoag @christophebedard @MCFurry @fujitatomoya