Skip to content

Fix segfault by adding __enter__ and __exit__ to Waitable (backport #761)#998

Merged
tfoote merged 2 commits intofoxyfrom
mergify/bp/foxy/pr-761
Sep 1, 2022
Merged

Fix segfault by adding __enter__ and __exit__ to Waitable (backport #761)#998
tfoote merged 2 commits intofoxyfrom
mergify/bp/foxy/pr-761

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify bot commented Aug 30, 2022

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

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

@sloretz @mergify
Add __enter__ and __exit__ to Waitable (#761)
71d95c4 
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 mergify bot mentioned this pull request Aug 30, 2022
Merged
@tfoote
FIx diff for foxy local variables
e51f918 
Signed-off-by: Tully Foote <tfoote@osrfoundation.org>
@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 30, 2022

This is causing users unexplained segfaults in foxy. This patch has been verified to solve the solution for them.

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 30, 2022 •
edited
Loading

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

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

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 30, 2022 •
edited
Loading

@ros-pull-request-builder restest this please

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 31, 2022

@ros-pull-request-builder retest this please

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 31, 2022 •
edited
Loading

Looks like the tests are flakey: https://build.ros2.org/job/Fpr__rclpy__ubuntu_focal_amd64/77/ failed but https://build.ros2.org/job/Fpr__rclpy__ubuntu_focal_amd64/78 passed on the same commit.

Test Result (5 failures / +5)projectroot.test_action_serverprojectroot.test_timerrclpy.rclpy.test.test_action_server.TestActionServer.test_different_feedback_type_raisesrclpy.rclpy.test.test_timer.test_number_callbacks[0.001]rclpy.rclpy.test.test_timer.test_cancel_reset[0.001]

Test Result (5 failures / +5)
projectroot.test_action_server
projectroot.test_timer
rclpy.rclpy.test.test_action_server.TestActionServer.test_different_feedback_type_raises
rclpy.rclpy.test.test_timer.test_number_callbacks[0.001]
rclpy.rclpy.test.test_timer.test_cancel_reset[0.001]

@tfoote
Copy link
Copy Markdown
Contributor

tfoote commented Aug 31, 2022

The windows failure looks independent and likely executor related.


Starting >>> ros2bag
13:52:07 ============================= test session starts =============================
13:52:07 platform win32 -- Python 3.8.3, pytest-7.1.2, pluggy-1.0.0
13:52:07 cachedir: C:\ci\ws\build\ros2bag\.pytest_cache
13:52:07 rootdir: C:\ci\ws\src\ros2\rosbag2\ros2bag, configfile: pytest.ini
13:52:07 plugins: ament-copyright-0.9.7, ament-flake8-0.9.7, ament-lint-0.9.7, ament-mypy-0.9.7, ament-pep257-0.9.7, ament-xmllint-0.9.7, launch-testing-0.10.8, launch-testing-ros-0.11.6, colcon-core-0.10.0, cov-3.0.0, mock-3.8.2, repeat-0.9.1, rerunfailures-10.2, timeout-2.1.0
13:52:07 collected 11 items
13:52:07 
13:52:07 test\test_api.py ......                                                  [ 54%]
13:52:07 test\test_copyright.py .                                                 [ 63%]
13:52:08 test\test_flake8.py .                                                    [ 72%]
13:52:08 test\test_pep257.py .                                                    [ 81%]
13:52:09 test\test_play_qos_profiles.py .                                         [ 90%]
13:52:42 test\test_record_qos_profiles.py R                                       [100%]R [100%]F [100%]
13:52:42 
13:52:42 ================================== FAILURES ===================================
13:52:42 ___________________ launch tests: test_record_qos_profiles ____________________
13:52:42 
13:52:42 ====================================================================================================================================================================================================================
13:52:42 FAIL: TestRos2BagRecord.test_incomplete_qos_profile
13:52:42 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
13:52:42 Traceback (most recent call last):
13:52:42   File "C:\Python38\lib\shutil.py", line 613, in _rmtree_unsafe
13:52:42     os.unlink(fullname)
13:52:42 PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\tmpcqvz6and\\ros2bag_test_incomplete_0.db3'
13:52:42 
13:52:42 During handling of the above exception, another exception occurred:
13:52:42 
13:52:42 Traceback (most recent call last):
13:52:42   File "C:\Python38\lib\tempfile.py", line 803, in onerror
13:52:42     _os.unlink(path)
13:52:42 PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\tmpcqvz6and\\ros2bag_test_incomplete_0.db3'
13:52:42 
13:52:42 During handling of the above exception, another exception occurred:
13:52:42 
13:52:42 Traceback (most recent call last):
13:52:42   File "C:\ci\ws\src\ros2\rosbag2\ros2bag\test\test_record_qos_profiles.py", line 87, in test_incomplete_qos_profile
13:52:42     assert not matches, print('ros2bag CLI did not produce the expected output')
13:52:42   File "C:\Python38\lib\tempfile.py", line 828, in __exit__
13:52:42     self.cleanup()
13:52:42   File "C:\Python38\lib\tempfile.py", line 832, in cleanup
13:52:42     self._rmtree(self.name)
13:52:42   File "C:\Python38\lib\tempfile.py", line 814, in _rmtree
13:52:42     _shutil.rmtree(name, onerror=onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 737, in rmtree
13:52:42     return _rmtree_unsafe(path, onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 615, in _rmtree_unsafe
13:52:42     onerror(os.unlink, fullname, sys.exc_info())
13:52:42   File "C:\Python38\lib\tempfile.py", line 806, in onerror
13:52:42     cls._rmtree(path)
13:52:42   File "C:\Python38\lib\tempfile.py", line 814, in _rmtree
13:52:42     _shutil.rmtree(name, onerror=onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 737, in rmtree
13:52:42     return _rmtree_unsafe(path, onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 596, in _rmtree_unsafe
13:52:42     onerror(os.scandir, path, sys.exc_info())
13:52:42   File "C:\Python38\lib\shutil.py", line 593, in _rmtree_unsafe
13:52:42     with os.scandir(path) as scandir_it:
13:52:42 NotADirectoryError: [WinError 267] The directory name is invalid: 'C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\tmpcqvz6and\\ros2bag_test_incomplete_0.db3'
13:52:42 
13:52:42 
13:52:42 ====================================================================================================================================================================================================================
13:52:42 FAIL: TestRos2BagRecord.test_qos_simple
13:52:42 --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
13:52:42 Traceback (most recent call last):
13:52:42   File "C:\Python38\lib\shutil.py", line 613, in _rmtree_unsafe
13:52:42     os.unlink(fullname)
13:52:42 PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\tmp5pcgi6gw\\ros2bag_test_basic_0.db3'
13:52:42 
13:52:42 During handling of the above exception, another exception occurred:
13:52:42 
13:52:42 Traceback (most recent call last):
13:52:42   File "C:\Python38\lib\tempfile.py", line 803, in onerror
13:52:42     _os.unlink(path)
13:52:42 PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\tmp5pcgi6gw\\ros2bag_test_basic_0.db3'
13:52:42 
13:52:42 During handling of the above exception, another exception occurred:
13:52:42 
13:52:42 Traceback (most recent call last):
13:52:42   File "C:\ci\ws\src\ros2\rosbag2\ros2bag\test\test_record_qos_profiles.py", line 75, in test_qos_simple
13:52:42     assert not matches, print('ros2bag CLI did not produce the expected output')
13:52:42   File "C:\Python38\lib\tempfile.py", line 828, in __exit__
13:52:42     self.cleanup()
13:52:42   File "C:\Python38\lib\tempfile.py", line 832, in cleanup
13:52:42     self._rmtree(self.name)
13:52:42   File "C:\Python38\lib\tempfile.py", line 814, in _rmtree
13:52:42     _shutil.rmtree(name, onerror=onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 737, in rmtree
13:52:42     return _rmtree_unsafe(path, onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 615, in _rmtree_unsafe
13:52:42     onerror(os.unlink, fullname, sys.exc_info())
13:52:42   File "C:\Python38\lib\tempfile.py", line 806, in onerror
13:52:42     cls._rmtree(path)
13:52:42   File "C:\Python38\lib\tempfile.py", line 814, in _rmtree
13:52:42     _shutil.rmtree(name, onerror=onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 737, in rmtree
13:52:42     return _rmtree_unsafe(path, onerror)
13:52:42   File "C:\Python38\lib\shutil.py", line 596, in _rmtree_unsafe
13:52:42     onerror(os.scandir, path, sys.exc_info())
13:52:42   File "C:\Python38\lib\shutil.py", line 593, in _rmtree_unsafe
13:52:42     with os.scandir(path) as scandir_it:
13:52:42 NotADirectoryError: [WinError 267] The directory name is invalid: 'C:\\Users\\ContainerAdministrator\\AppData\\Local\\Temp\\tmp5pcgi6gw\\ros2bag_test_basic_0.db3'
13:52:42 ---------------------------- Captured stdout call -----------------------------
13:52:42 [INFO] [launch]: All log files can be found below C:\Users\ContainerAdministrator\.ros\log\2022-08-31-20-54-27-412284-21ac05cf785b-14660
13:52:42 [INFO] [launch]: Default logging verbosity is set to INFO
13:52:42 [INFO] [ros2bag-cli-3]: process started with pid [8808]
13:52:42 [ros2bag-cli-3] [ERROR] [ros2bag]: Time overrides must include both seconds (sec) and nanoseconds (nsec).

@tfoote tfoote requested a review from sloretz August 31, 2022 21:36
@jacobperron
Copy link
Copy Markdown
Member

jacobperron commented Sep 1, 2022

The Windows failures are unrelated. Here's a baseline for Windows I ran not too long ago: https://ci.ros2.org/job/ci_windows/17425/

sloretz
sloretz approved these changes Sep 1, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

Looks backportable to me.

@tfoote tfoote merged commit e4e34e7 into foxy Sep 1, 2022
@delete-merged-branch delete-merged-branch bot deleted the mergify/bp/foxy/pr-761 branch September 1, 2022 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jacobperron jacobperron jacobperron approved these changes

@sloretz sloretz sloretz 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.

3 participants

@tfoote @jacobperron @sloretz