Created attachment 13805 [details] [review]
Patch to implement the enhancement

This seems to be key to the implementation:

To accomplish this, one of the two objects holds a reference to
+//    the other at any given time. The object holding the reference is
+//    the anchor. With one exception, there will always be at least one
+//    external reference to the anchor. The exception is when a wrapper
+//    is newly constructed and has a reference to a GObject, but has not
+//    yet been referenced itself.
+//
+//    Normally, the wrapper is the anchor. The only time that the
+//    GObject is the anchor is when there are no external references to
+//    the wrapper (except for the case described above).
+//    
+//    If the wrapper is the anchor and the last external reference to it
+//    is released, then the GObject becomes the anchor. The GObject is
+//    given a reference to the wrapper and the wrapper's reference to
+//    the GObject is released. This happens in the wrapper's
+//    unreference() method. 
+//
+//    If the GObject is the anchor and an external reference to the
+//    wrapper is acquired, then the wrapper again becomes the anchor.
+//    The wrapper acquires a reference to the GObject and the reference
+//    to the wrapper is removed from the GObject. This happens in the
+//    wrapper's reference() method.
+//
+//    If the GObject is the anchor and its reference count goes to zero,
+//    then the GObject is destroyed and destroy_notify_callback_() is
+//    invoked.
+//
+//    The destroy_notify_callback_() acquires a temporary reference to
+//    the wrapper (which makes it the anchor) and calls the wrapper's
+//    destroy_notify_() virtual method. The destroy_notify_() method
+//    clears the wrapper's pointer to the GObject. Upon return from
+//    destroy_notify_() the temporary reference to the wrapper is
+//    released. Since it is the last reference to the wrapper, the
+//    wrapper is then deleted.

Frankly, that sounds complex and I must admit that I haven't taken the
time to read it all as slowly as I should. I'd like it if it could be
described more concisely.

Let's consider this for glibmm 2.4 and try to make a patch for it if
we like it.

Minor notes:
- I have already removed RefPtr::is_null() in glibmm 2.4.
- Be careful when applying the patch - it includes documentation
changes that might not be wanted and seems to remove essential debug
output.

It's really not that complex in the implementation. The code that
primarily implements it is confined to a few short, simple functions.
It might be possible to simplify it by having the GObject hold a
RefPtr<> to the wrapper at all times, which is what I did in the
IOChannel implementation. But that involves the overhead of allocating
another object on the heap and I wanted to avoid that.

There are a few places where I removed GTKMM_DEBUG_REFERENCE and
GTKMM_DEBUG_UNREFERENCE macro calls, because what was being referenced
or unreferenced at those points was the GObject, not the
Glib::ObjectBase. It may be that a second pair of macros would be
useful to provide debug output for the two separate cases. I tried to
leave other debugging output in place.

The reason why it seems complex, perhaps, is that the Glib::ObjectBase
implementation is doing the necessary work to hide the complexity of
the GTK+ reference counting implementation. To the end user, things
are simpler and more consistent because they don't have to worry about
the eccentric GTK+ behavior.

Why did you remove the cpp_destruction_in_progress_ bool and checks?

Have you tried all the tests and the gtk-demo? Do they run and quit
without warnings or segfaults?

Why did you also make Glib::MainContext inherit from GMainContext?
Along with all the irrelevant or trivial documentation and other
changes it is very hard to process this patch.

I've tried all the examples and the gtk-demo. Yes, they run with no
warnings or segfaults.

The cpp_destruction_in_progress got moved (and shortened to
in_destructor) into Gtk::Object since that is the only place it is
actually set. I provided an accessor function (is_in_destructor) for
testing the value. It is tested in widget.ccg, as it was before.

As for the comments and documentation, the idea was to fully document
all of what I was doing for future reference, rather than going for a
smaller patch and making things harder for someone to figure out later.

As to the Glib::MainContext inheriting from GMainContext, it neatly
avoids using reinterpret_cast, a construct which Stroustrup considers
highly undesirable. It also, in my opinion, makes it clearer that
Glib::MainContext is pretending to be a GMainContext. The same is true
of Glib::MainLoop. Neither of these is actually wrapping the GLib
objects, they are extending them.

Created attachment 13875 [details] [review]
glibmm_object.patch

This patch, glibmm_object.patch, simplifies things a bit and shows
just the lifecycle changes, though it also moves some methods around
without changing them.

I've looked at this again and I realise that I don't understand.
Sorry, I'm stupid. Can you try explaining this anchor thing again?

I wouldn't attribute a lack of understanding to stupidity. It took me
a little while to come up with the model. It's actually not that
complicated, but you need to get the right perspective, I guess. If I
could whiteboard it, that would probably help.

The basic problem that needed to be solved is that there are two
objects, closely coupled, that are both reference counted. Certain
constraints needed to be obeyed in describing the interactions between
them. Foremost, they need to have separate reference counts, because
the GTK+ model of reference counting is incompatible with the
conventional C++ model. First, some definitions:

1) The C object is the "GObject". It is the owned object.
2) The C++ object is the "wrapper". It owns an internal reference to
the GObject.
3) An "internal reference" is a reference to one of the two objects,
held by the other object.
4) An "external reference" is a reference to one of the two objects
that is not held by the other object. In the case of the wrapper, an
external reference is embodied by a RefPtr. In the case of the
GObject, an external reference would be held by some GTK+ entity.

Next, some constraints:

1) Both objects need to exist so long as there are external references
to either of them. This is so that the wrap() function will always
return the same wrapper.
2) Either object can keep the other alive by maintaining an internal
reference.
3) The two objects cannot hold mutual internal references, because
they would then keep each other alive indefinitely.

So the problem became, under what conditions should either of the
objects hold an internal reference in order to satisfy the first and
third constraints? In order to make this easier to determine, I came
up with the designation of the "anchor" object:

4) Only one object is the anchor object. It is the responsibility of
the anchor object to keep the other alive when it might otherwise die.
5) So long as there are external references, there will be at least
one external reference to the anchor.
6) Only the anchor object has an internal reference.

Because the wrapper starts with an internal reference (to the GObject)
and keeps that reference almost all the time, it makes sense for the
wrapper to be the anchor under most conditions. The only time that the
wrapper cannot be the anchor is when there are no more external
references to it (because of constraint 5).

So basically, when there are external references to the wrapper, it is
the anchor and holds an internal reference to the GObject. When (and
only when) there are no external references to the wrapper, then the
GObject is the anchor and holds an internal reference to the wrapper.
The manner in which the GObject "holds" the internal reference is an
implementation detail, but it is useful to note that all of the work
actually happens in member functions of the wrapper, because we cannot
 change the implementation of the GObject.

An important consequence of making the GObject the anchor whenever
there are no external references to the wrapper is that the GObject
will always be the anchor when there are no more external references
at all. Hence, the GObject will always be destroyed first, which will
in turn cause the destruction of the wrapper.

One must of course be careful of the order in which references are
acquired and released in the wrapper implementation.

The implementations of Glib::IOChannel and Glib::Source use a slight
modification of this basic scheme which finesses constraint 6, because
their "gobject" is an instance of a local C++ class rather than an
opaque C structure.

Of course, things get more complicated in Gtk::Object, but I won't go
into that at the moment, other than to note that there are a few hooks
built into Glib::ObjectBase to facilitate the implementation of
Gtk::Object.

Does this help any?

I'm not ignoring this, but I am waiting until Daniel Elstner has a
look at it. It will probably be considered more when glibmm 2.4
development starts properly, when a glib 2.3.0 is released.

I like this idea more and more, but I think it's too high risk now.
Maybe next time we break ABI. Sorry for not using your great work yet.

Not reading in detail, but:
There is no SigC::RefPtr<> equivalent in libsigc++-2.0. We removed the memory
management functionality because libsigc++ is a signal/slot library and no smart
pointer library.

I feel tempted to close this bug as Obsolete. Objections?

The patches can't be used as is. They are 12 years old, and don't fit with
the present code. Now there is no SigC::Object and no GtkObject (although
there is still a Gtk::Object). Glib::Source has been very much changed. Etc.

Yes, this isn't going to happen.