-
Notifications
You must be signed in to change notification settings - Fork 330
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
Fix leak(#480) #481
Fix leak(#480) #481
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit, and then I'm happy to approve and run CI.
@@ -32,7 +32,7 @@ int main(int argc, char * argv[]) | |||
rclcpp::Logger logger = rclcpp::get_logger(LINKTIME_COMPOSITION_LOGGER_NAME); | |||
rclcpp::executors::SingleThreadedExecutor exec; | |||
rclcpp::NodeOptions options; | |||
std::vector<class_loader::ClassLoader *> loaders; | |||
std::vector<std::shared_ptr<class_loader::ClassLoader>> loaders; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there is only ever going to be one owner, I would suggest using a std::unique_ptr
here.
Signed-off-by: y-okumura-isp <y-okumura@isp.co.jp>
Thank you for comment. I updated PR. http://wiki.ros.org/class_loader may be the reason.
|
Windows CI is failing for a completely unrelated reason. I think this is safe enough to put it without that, so I'm going to go ahead and merge. |
Fix #480.
Although there are ASAN errors related to class_loader, this PR reduces one error.
Signed-off-by: y-okumura-isp y-okumura@isp.co.jp