Re: [ros-users] [ros-pkg] #4887: Extend pluginlib with more …

>> * 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