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

Enable on demand unloading #39

Closed

Conversation

rhaschke
Copy link

@rhaschke rhaschke commented Apr 3, 2016

Fixes #37, issue1.cpp. Relates to #38, which is fixed by ros/class_loader#34.

@rhaschke
Copy link
Author

rhaschke commented Apr 3, 2016

The problem is definitely in libclass_loader only, which requires that the ClassLoaders stay in place as long as objects do.

@rhaschke rhaschke force-pushed the enable-on-demand-unloading branch from fff53f6 to 9e2cff2 Compare April 3, 2016 14:02
@rhaschke
Copy link
Author

rhaschke commented Apr 3, 2016

With ros/class_loader#34 the unit tests should succeed and on-demand loading/unloading should work again.

@mikaelarguedas
Copy link
Member

Hi @rhaschke
Given that ros/class_loader#34 does fix the library loading issue for ondemand loading/unloading but doesn't solve the unloading problem when the ClassLoader is destroyed;
Do you think that enabling ondemand loading/unloading in the pluginlib's ClassLoader still makes sense ? It will result in the same SEVERE WARNING and not unloading the library when ClassLoader destructor is called. As a result #37 issue1.cpp will not be fixed by this PR. Is that right?

@rhaschke
Copy link
Author

@mikaelarguedas
Indeed, one gets the SEVERE WARNING when trying to destroy the ClassLoader when objects are still in use. Hence ros/class_loader#34 doesn't fix both issues of #37.
I was thinking about to drop PR #39 too, because there is not benefit yet in using on-demand unloading of libraries: Either there is the SEVER WARNING or a segfault on exit of the program...
Hence, feel free to close this PR without merging.

@mikaelarguedas
Copy link
Member

@rhaschke I will then close this PR. The related issue can be kept open if anyone wants to take over and offer a solution to fix them.
Thanks for your work,

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