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

Unregister typesupport rather than delete #416

Closed
wants to merge 1 commit into from

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Jul 29, 2020

There was a use-after-free that I discovered while adding fault injection tests to rcl_action over at ros2/rcl#730. Somehow rcl_action's typesupport for its client and subscription were being duplicated. When it was cleaned up during this fail block in create_subscription, it would try to delete it again over in rmw_client_fini. Instead, I believe it needs to unregister it, which also deletes it, so that it won't get deleted twice. rmw_client.cpp already does this with unregister rather than deleting the pointer directly.

Before-and-after unit tests will have to wait until tomorrow.

Signed-off-by: Stephen Brawner brawner@gmail.com

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Contributor Author

brawner commented Aug 12, 2020

Builds and tests of the following packages rcutils, rcl_logging_spdlog, rosidl_typesupport_c, rosidl_typesupport_cpp, rcl, rcl_action, rcl_lifecycle, rosidl_runtime_c, rmw_dds_common, rmw

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

@brawner brawner requested a review from hidmic August 19, 2020 19:27
@hidmic
Copy link
Contributor

hidmic commented Aug 19, 2020

@brawner I think #419 fixed this (among other things), no?

@brawner
Copy link
Contributor Author

brawner commented Aug 19, 2020

I think you're correct.

@brawner brawner closed this Aug 19, 2020
@hidmic hidmic deleted the brawner/rmw_fastrtps-init-fini-bugs branch August 19, 2020 20:42
@hidmic hidmic restored the brawner/rmw_fastrtps-init-fini-bugs branch August 19, 2020 20:42
@hidmic
Copy link
Contributor

hidmic commented Aug 19, 2020

@brawner are you keeping the branch for anything in particular?

@brawner brawner deleted the brawner/rmw_fastrtps-init-fini-bugs branch August 19, 2020 20:44
@brawner
Copy link
Contributor Author

brawner commented Aug 19, 2020

No, deletebot didn't want to delete it I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants