>> * calling "unloadLibraryForClass" with an unavailable class: >> I would propose to throw an exception (introduce LibraryUnloadException) >> which would be similar to what happens for "loadLibraryForClass". >> Rational: this is definitely wrong usage of pluginlib as the developer >> could verify the availability of the class before. >> >> * calling "unloadLibraryForClass" for a not loaded class (counter doesn't >> exist or is zero): >> I would propose to also throw a LibraryUnloadException. >> Rational: this is also wrong usage of pluginlib as it does not follow the >> convention for one unload call for each previous call of load. > > This looks like a fitting implementation. The exception parallels the > LibraryLoadException, and should inherit similarly form > pluginlib::Exception ok, i will update the patch accordingly. >> Now we get to the nasty cases: >> >> * calling "unloadLibraryForClass" for a class which is loaded multiple times >> (counter> 1): >> * calling "unloadLibraryForClass" for a class which is loaded only once >> (counter == 1): >> The developer might want to distinguish both cases as in the first case >> reloading the library will NOT load the newest version of the code. >> I would propose to use the return value for this: false in the first case, >> true for the second. >> >> BUT the developer can never be 100% sure about this. >> Consider the following scenario with two libraries A and B: >> - load A >> - load B (which might unintentionally bind to symbols from A) >> - unload A >> Even after calling "dlclose" (which happens inside of Poco when unloading A) >> the library A will stay in memory due to the symbols still in use. >> Loading A again will therefore NOT load the newest version of the code. >> >> I don't think there is anything we can do about it. >> The developer of the libraries must assure that this cross-library binding >> to symbols does not happen. >> (E.g. by stripping all non-Poco symbols from the library - I will add an >> example for this stripping to the ROS GUI rep soon) >> >> First, did I get it right? >> (This is all written from what I remember from several tests half a year >> ago) >> Do you think this return value would be a reasonable solution? >> > > I would probably rather an exception as exceptions are already used > for the other error cases. The developer should already be inside a > try block, and they could then choose either to catch a specific > exception or just warn the user that the unload failed by catching the > generic exception. For which case would you propose to use exceptions here? Calling "unloadLibraryForClass" which is loaded once/multiple times should not throw an exception. The return value indicating how many times the library is still loaded might be useful. The following would be valid code: load(a); load(a); unload(a); unload(a); The other case - when unloading the library is not "effective" (because some symbols are still in use) - is imho not detectable. So throwing an exception here is not an option. Dirk