Re: [gtkmm] Glib::RefPtr construction

[murrayc t-online de (Murray Cumming)]

On Wed, 2003-01-08 at 01:50, Michael Johnson wrote:
> Murray Cumming wrote:
> > On Tue, 2003-01-07 at 22:08, Michael Johnson wrote:
> >   
> > > In reading through the source for Glib::RefPtr I note that the constructor
> > > 
> > >     RefPtr<T_CppObject>::RefPtr(T_CppObject* pCppObject)
> > > 
> > > Does not increase the reference count of the referenced object,

As I said before, that's because GTK objects are created with a
reference count of 1. If we increase it to 2 then the object will not be
deleted during the RefPtr<>'s final destructor. But that's the whole
point of RefPtr<>
 
> > >  though 
> > > all other means of assigning a value to a RefPtr (including the 
> > > assignment operator that takes a pointer)
> > >  do increase the reference 
> > > count. Is this intentional, or is it an accidental omission?

Yes, maybe that operator=(T_CppObject*) should be the same as the
constructor. I'm not sure when that operator= is used anyway. Maybe it's
safe to change it.

> > The pCppObject is normally created with a refcount of 1, and the final
> > RefPtr<> destructor unrefs it to 0, causing pCppObject to be deleted.
> > 
> >   
> I realize I am a newcomer to the list, and I do not wish to step on
> any toes, so please consider the following discussion in the spirit in
> which it is offered, as constructive discourse. In my professional
> capacity, I have in the past implemented a large, complex system of
> classes that used smart-pointer management of reference-counted
> objects and as a result I have learned some things from practical
> experience.
> 
> In my opinion, the behavior described above appears to be dangerous
> and error-prone. Especially in light of the fact that SigC::Object,
> which is perfectly capable of being managed by RefPtr, does not behave
> this way.

RefPtr<> is not intended for this purpose. RefPtr is for use with
Glib::Object-derived objects only. If you can add some code to prevent
the template from compiling with anything else then I would welcome the
patch.

SigC::Ptr might be more useful for your purposes. I haven't used it. I
normally suggest that people use a more general smartpointer for more
general needs.

>  It starts with a reference count of zero. Consider the following
> samples of code:
> 
>     Glib::RefPtr<Foo> rpFoo;
>     return (rpFoo = new Foo());
> 
> Versus:
> 
>     return Glib::RefPtr<Foo>(new Foo());
> 
> or even:
> 
>     Glib::RefPtr<Foo> rpFoo = new Foo();
>     return rpFoo;
>
> These three code examples, which one would reasonably expect to behave
> the same, in fact yield different results. The first case will result
> in a reference count that is one higher than the other two cases. This
> is non-intuitive.

This is a repetition of your first point about operator=(), isn't it?

> It also seems to me that constructing the object with a reference
> count of 1 is conceptually incorrect.

Well that's GTK+, not gtkmm. That's just the start of the GTK+
reference-counting problems, all of which are simplified by the gtkmm
API.

>  At the time of construction, no "reference" to the object exists,
> therefore the reference count should be zero. There will not in fact
> be a reference to the object until a RefPtr (which embodies the
> reference) is constructed with or assigned a pointer to the object.
> There is also the question of appropriate assignment of roles and
> responsibilities. A reference-counted object has a reference count,
> but does not itself manage that reference count. It merely provides a
> means for other objects (which embody references) to manage the
> reference count. It is the role of the reference class to manage the
> reference count of objects being referenced.

Are you complaining that GObject holds the reference instead of the
RefPtr<>? Well, that's how it is. If you can provide a patch that makes
RefPtr<> more useful then I would be interested to see it.

>  It makes sense then that any method of the reference class that
> changes the value of the reference should participate in the
> management of the reference count.
> 
> Looking at Glib::ObjectBase, there is probably no reason why its
> overrides of reference() and unreference() could not in turn call the
> SigC::ObjectBase reference() and unreference() methods. By default,
> those methods will not delete the object when the reference count goes
> to zero.

What would they do? Increase a reference count? What use would the 2nd
reference count number do?

>  This only happens if the set_manage() method is called, so it is
> normally safe to construct objects derived from Glib::ObjectBase on
> the stack. It then becomes simpler for classes derived from
> Glib::ObjectBase to enable lifetime management by reference counting.
> They don't have to override reference() and unreference(), they merely
> need to invoke set_manage() in their constructor.

This is not the purpose of set_manage() as far as I know.

>  Nor do they need to (redundantly, because it is already being done by
> SigC::ObjectBase) include a reference-count data member.
> 
> This has advantages. Consider the possibility of reference-counted
> objects being shared across multiple threads. Overlapping calls to the
> reference() and unreference() methods from different threads must be
> guarded against. The simplest way to do this is to use an atomic
> counter so that increment or decrement of the counter is thread-safe.
> If many classes in gtkmm re-implement reference counting, then all of
> those classes must be changed in order to make gtkmm thread-safe. If
> those same classes used the underlying reference counting in
> SigC::ObjectBase then only SigC::ObjectBase would need to be made
> thread-safe in order for all classes derived from it to be in turn
> thread-safe.
> 
> Does anyone else have any thoughts on this?

There seems to be 1 possible bug here and a great deal of talk. I would
be interested in 
a) Investigation of the effects of changing that operator=().
b) A patch to implement your ideas, which you have tested.

-- 
Murray Cumming
murray usa net
www.murrayc.com