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

[Chris Shoemaker]

On Thu, Jun 02, 2005 at 05:29:06PM -0400, David Hampton wrote:
> On Thu, 2005-06-02 at 16:59 -0400, Chris Shoemaker wrote:
> > 1) The gtk_destroy signal handler (gnc_customer_window_destroy_cb) does
> > NOT emit the CM event.  
> > (It should be calling gnc_close_gui_component(cw->component_id);)
> > 
> > 2) The CM destroy handler (gnc_customer_window_close_handler) does NOT
> > destroy the dialog context.  Instead, that's actually done inside the
> > gtk_destroy signal handler.
> > 
> > 
> > The biggest hint that this is an abuse of the CM api is the fact that
> > the close handler is not explicitly unregistering the component with
> > the CM, instead relying on it happening as a side-effect of the
> > gtk_destroy signal.  If the close handler wanted to do anything with
> > the component after it destroyed the dialog (like, say, read the
> > non-gui state) it would be screwed because the whole component has
> > been freed!
> > 
> > I think all the confusion goes away when you do something like this:
> > [warning: not even compile tested]
> > 
> > // This is the gtk_destroy signal handler
> > void
> > gnc_customer_window_destroy_cb (GtkWidget *widget, gpointer data)
> > {
> >   CustomerWindow *cw = data;
> > 
> >   gnc_close_gui_component(cw->component_id);
> > 
> > }
> > 
> > // This is the CM close handler
> > static void
> > gnc_customer_window_close_handler (gpointer user_data)
> > {
> >   CustomerWindow *cw = user_data;
> >   GncCustomer *customer = cw_get_customer (cw);
> > 
> >   gnc_suspend_gui_refresh ();
> > 
> >   if (cw->dialog_type == NEW_CUSTOMER && customer != NULL) {
> >     gncCustomerBeginEdit (customer);
> >     gncCustomerDestroy (customer);
> >     cw->customer_guid = *xaccGUIDNULL ();
> >   }
> > 
> >   gnc_unregister_gui_component (cw->component_id);
> > 
> >   gtk_widget_destroy (cw->dialog);
> >   cw->dialog = NULL;
> >   g_free (cw);
> > 
> >   gnc_resume_gui_refresh ();
> > }
> > 
> > Makes a heck of a lot more sense to me.  David?
> 
> I think this will work because gtk_widget_destroy correctly handles
> recursive invocation.  If the widget is already in the process of being
> destroyed then the call to gtk_widget_destroy from
> gnc_customer_window_close_handler is a noop, otherwise it will actually
> destroy the dialog.

That makes sense since "gtk_widget_destroy" is supposed to mean "break
all your references".  If they're already broken, re-calling does
nothing.  

I'm not sure if the destroy-signal will get re-delivered in the
recursive case, but I would hope that it wouldn't matter since I would
expect the CM to ignore any events after the un-registration, whether
the events are cached and then discarded because they arrived after
the unregistration, or were discarded immediately because the
component had already been unregistered.

-chris