fix on demand unloading

[rhaschke]

On-demand-unloading was broken because:

• loading always needs to be done, otherwise the classes from the lib cannot be known
• MultiLibraryClassLoader shouldn't unload its libs when on-demand-unloading is enabled

This relates to ros/pluginlib#37 and fixes ros/pluginlib#38.

[rhaschke]

Hm. Didn't run the test before. Will have a short look.

[rhaschke]

Ah, obviously my changes need to be applied to MultiLibraryClassLoader, not the base ClassLoader.
I will fix that and rebase.

[rhaschke]

IMHO the class_loader library has a fundamental design flaw: The unloading of libraries is tight to the existance of a ClassLoader. However, for example in liburdf, the loader could be destroyed before its created plugin objects. I don't see a reason, why this shouldn't be possible conceptually.

Of course, it's hard(er) to implement: Knowledge about a loaded library (load_ref_count, plugin_ref_count) should be stored separate from the ClassLoader. Actually this is already the case for some info (MetaObjectVector + LibraryVector), but not yet the mentioned variables. The plugin deleter (onPluginDeletion) should accordingly check against those global variables instead of ClassLoaders.

Additionally I suggest to keep the library loaded while a ClassLoader is instantiated. Otherwise the library might get loaded and unloaded for every single object instance (if this gets destroyed in between).

Furthermore, all the static global variables should be integrated in a singleton shared_ptr class, such that destruction order can be controlled. Having this singleton a shared_ptr, also allows to deal with static ClassLoader instances (as in liburdf): Only if all ClassLoader instances are destroyed, the actual singleton pointer should be released. This will finally solve #33.

As you see, I have some implementation ideas in mind. However, I don't have time to work on this in the near future. The current PR fixes on-demand-loading for MultiLibClassLoader, but still requires the class loaders to be kept instantiated as long as as plugin objects exist.

[mikaelarguedas]

Hi Robert,
Sorry to keep you waiting.

This seems like a reasonable incremental improvement. I totally agree with you that there is room for improvement on the way ClassLoader and loaded classes are tied together.
As you stated this would require some work and I don't know if anybody will have time to address this in the near future.

Thanks for your work!

[mikaelarguedas]

@dirk-thomas any comment about this fix ?

[dirk-thomas]

LGTM