On Thu, Feb 9, 2012 at 12:55 PM, Dirk Thomas <dthomas@willowgarage.com>wrote:

>  While I like boost::shared_ptr, it is not always possible to use it
>> nicely.
>>
>> In rviz I have plugin classes which are subclasses of QWidget (from Qt
>> GUI library).  These objects are automatically deleted by Qt when their
>> parent widgets are deleted.  I can be notified of their
>> deletion by Qt, and call an "unload" or "decrement-reference-count"
>> function, but it is not easy to use these QWidget pointers inside
>> boost::shared_ptr without getting double-deletion crashes.
>>
>
> That sounds like a very reasonable use case.
> Automatically deleting and unloading is a nice feature but might not
> always be desired.
> Having the option to get a raw pointer (as before) and take care of
> garbaging stuff manually would be great.


I don't like this, but don't see a clearly better solution.

So we have the problem that the shared_ptr is actually managing two
resources - the lifetime of the object, and the lifetime of the loaded
library (which must outlive all objects using it). The question is can we
decouple those in a sane way. If we give up knowledge of the object
lifetime, we also give up the ability to automatically manage the library
lifetime.

One slightly different approach:

T* getInstance(const std::string& lookup_name, LibraryHandle& handle);

Where the additional output LibraryHandle (could just be a shared_ptr
typedef) represents the library lifetime, and decrements the lib ref count
when it's destroyed. The user is responsible for making sure handle
outlives the returned object. The advantages are that we don't have to
expose [un]loadLibraryForClass() functions forever, and we make
responsibility for the library lifetime explicit.

 pluginlib::ClassLoader could have two interfaces, one which gives a
>> shared_ptr and manages the reference count for the library itself, and
>> another which gives a raw pointer and increments the ref
>> count and also has a decrement-ref-count function.
>>
>
> I propose the following names to make clear what the difference is (of
> course with a more detailed doc-string):
>
>  boost::shared_ptr<T> getManagedInstance(const std::string& lookup_name)
>
>  T* getInstance(const std::string& lookup_name)
>

In any case I'd reverse the naming convention - have getInstance() return a
shared_ptr and, say, getUnmanagedInstance() return T*. We should make the
encouraged usage easier, and discourage the advanced/unsafe usage with a
longer name.

Cheers,
Patrick