[Gnucash-changes] Eliminate a double free of memory.

[David Hampton]

On Fri, 2005-06-03 at 15:24 -0400, Chris Shoemaker wrote:
> On Fri, Jun 03, 2005 at 02:16:46PM -0400, David Hampton wrote:
> > On Fri, 2005-06-03 at 11:11 -0400, Chris Shoemaker wrote:
> > 
> > > 1) It's an abuse of the documented gtk destroy api, which is only
> > > suppose to break references, NOT free anything.
> > 
> > We're reading the documentation differently.
> > 
> >     Because these references are released, calling gtk_object_destroy()
> >     should result in FREEING ALL MEMORY ASSOCIATED WITH AN OBJECT,
> >     unless some buggy code fails to release its references in response
> >     to the "destroy" signal.
> > 
> > Emphasis mine.  This begs the question of whether the gnucash data
> > structure that is intimately tied to the dialog is considered memory
> > associated with an object.  As an object implementor, I'd say no.  As a
> > gnucash developer, I'd say yes, since the majority of the fields in the
> > data structure point to widgets within the dialog, and all of these
> > pointers will be invalidated when the dialog is destroyed.
> 
> That question is not really relevant.  "should result in" is correct,
> but it means indirectly as a result of ref count dropping to 0.  Look
> at the api docs for the gtk_object_destroy() function:
> 
>     Emits the "destroy" signal notifying all reference holders that
>     they should release the GtkObject... The memory for the object
>     itself won't be deleted until its reference count actually drops to 0;
>     gtk_object_destroy() MERELY ASKS reference holders to RELEASE THEIR
>     REFERENCES, IT DOES NOT FREE THE OBJECT.
> 
> Emphasis mine.  It's not very ambiguous.

Lovely.  We're reading different parts of the same GtkObject
documentation and coming away with different interpretations.  

I'm trying to follow the execution through the gtk/glib sources, but I'm
not at all clear on how it works through.  As I see it the following
happens:

  gtk_object_destroy calls
    g_object_run_dispose calls
      g_object_ref (object)
        G_OBJECT_GET_CLASS (object)->dispose (object);
        gtk_window_dispose
          gtk_widget_dispose
            gtk_widget_unrealize
            gtk_object_dispose
              g_signal_emit (object, object_signals[DESTROY], 0);
                anything connected to destroy
                gtk_window_destroy
                  gtk_container_destroy
                    DESTROY ANY OBJECTS IN THIS CONTAINER
                    gtk_widget_real_destroy
                     gtk_object_real_destroy
                       g_signal_handlers_destroy (object);
                anything connected "after" to destroy
              g_object_real_dispose
                g_signal_handlers_destroy
      g_object_unref (object);

As far as I can tell, you'll still have the GtkWindow object itself
after the call to gtk_object_destroy finishes, but it will no longer
contain any child objects.  Remember every child widget holds a
reference on its parent, so if all reference holders must release their
references, then all child widgets must be removed.  While this
technically meets the statement that the memory of the object passed in
is not freed, its not really useful.  (The statement in the
documentation says nothing of what happens to contained objects.)  This
is the difference between calling g_object_unref knowing that the object
will eventually be destroyed when its refcount goes to zero, and
explicitly asking for the object to be destroyed by calling
gtk_object_destroy.   Please let me know if I'm reading the code wrong.
The signal code is a pain to parse.

> > You've clearly taken offense that he doesn't agree with you
> > that this is a heinous design issue that needs to be fixed immediately.
> 
> That's not really true.  "Offense" is too strong; It'd be more
> accurate to say that I'm "concerned" that he doesn't agree with me
> that this is a design issue _at_all_, and furthermore, (actually this
> is more of the real concern:) that this is just one example
> representative of a rather long list of (IMO) design "weaknesses"
> about which we disagree.  

I can't speak for Derek, but my guess would be that you two have very
different thresholds of what's important and what's not.  

David