Skip to content

Fix segfault by adding __enter__ and __exit__ to Waitable#761

Merged
sloretz merged 1 commit intomasterfrom
rclpy_760_segfault__waitable_context_manager
Apr 8, 2021
Merged

Fix segfault by adding __enter__ and __exit__ to Waitable#761
sloretz merged 1 commit intomasterfrom
rclpy_760_segfault__waitable_context_manager

Conversation

@sloretz
Copy link
Copy Markdown
Contributor

@sloretz sloretz commented Apr 7, 2021

Fixes #760

Also make QoS event waitable use it. This fixes a crash when another
thread is spinning on a destroyed subscription using cyclonedds.

I was never able to reproduce the crash with rqt_console, but I can confirm it fixes the crash with this MRE #760 (comment)

Also make QoS event waitable use it. This fixes a crash when another
thread is spinning on a destroyed subscription using cyclonedds.

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
@sloretz
Copy link
Copy Markdown
Contributor Author

sloretz commented Apr 7, 2021

CI (build: --packages-above-and-dependencies rclpy test: --packages-above rclpy)

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

@clalancette
Copy link
Copy Markdown
Contributor

As an FYI, I was able to reproduce the problem with the rqt_console plugin, and this seems to solve it for me.

@clalancette
Copy link
Copy Markdown
Contributor

@sloretz Do you think this will also fix #762 ?

@sloretz
Copy link
Copy Markdown
Contributor Author

sloretz commented Apr 7, 2021

@sloretz Do you think this will also fix #762 ?

It seems likely based on the investigation done on it.

mjeronimo pushed a commit to ros-visualization/rqt_topic that referenced this pull request Apr 7, 2021
ros2/rclpy#761 fixes this issue.

Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

Makes sense to me, thanks @sloretz !!

@sloretz
Copy link
Copy Markdown
Contributor Author

sloretz commented Apr 8, 2021

2 approvals and green CI, merging 🎉

@sloretz sloretz merged commit a75529d into master Apr 8, 2021
@delete-merged-branch delete-merged-branch bot deleted the rclpy_760_segfault__waitable_context_manager branch April 8, 2021 16:17
clalancette pushed a commit to ros-visualization/rqt_topic that referenced this pull request Apr 29, 2021
ros2/rclpy#761 fixes this issue.

Signed-off-by: Michael Jeronimo <michael.jeronimo@openrobotics.org>
@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 30, 2022

@Mergifyio backport foxy

mergify bot pushed a commit that referenced this pull request Aug 30, 2022
Also make QoS event waitable use it. This fixes a crash when another
thread is spinning on a destroyed subscription using cyclonedds.

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
(cherry picked from commit a75529d)
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 30, 2022

backport foxy

✅ Backports have been created

Details

tfoote added a commit that referenced this pull request Sep 1, 2022
Fix segfault by adding __enter__ and __exit__ to Waitable (backport #761)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An rcl_event_t might be destroyed while a waitset is still using it

5 participants