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

Chris Shoemaker c.shoemaker at cox.net
Fri Jun 3 09:10:21 EDT 2005


On Thu, Jun 02, 2005 at 10:15:18PM -0400, Chris Shoemaker wrote:
> On Thu, Jun 02, 2005 at 06:55:18PM -0400, David Hampton wrote:
> > On Thu, 2005-06-02 at 17:33 -0400, Derek Atkins wrote:
> > It might be possible to add a call to g_signal_connect_after(cw->dialog,
> > "destroy", gtk_widget_destroyed, &cw->dialog) when the dialog is created
> > to resolve this case.   If the window destruction completes before the
> > CM close function is called, then this new callback will have zeroed the
> > cw->dialog pointer.  
> > 
> > Of course, since the gnc_customer_window_close_handler function frees
> > cw, it would have to remove this callback so the original invocation
> > order we were discussing won't write to freed memory.  I don't know if
> > this will work though as I don't know if glib handles deleting callback
> > functions from within a callback.  It appears that glib builds an
> > ordered list of all handlers before calling the first handler, so the
> > above solution will not work for all cases.
> > 
> > I suppose this could be flipped and the gnc_customer_window_destroy_cb
> > callback could be connected "after" the gtk_widget_destroyed handler.
> > In this case if the dialog has been closed via the window manager close
> > button cw->dialog will always be NULL regardless of whether the CM
> > callback runs immediately or at some later time.  If the dialog has been
> > closed some other way, then cw->dialog will be non-NULL and this
> > function can call gtk_widget_destroy.  I think this works. 
> 
> Maybe there's an even easier way: sink and ref the dialog when it gets
> created, and then unref it only in the CM close handler, right before
> the gtk_destroy_widget() call.  Then you know the dialog won't be
> freed until you're done with it.

After thinking about this overnight, I think taking a ref is probably
the right thing to do.  After all, cw->dialog _is_ essentially now an
uncounted ref.  And ref counting is supposed to prevent the exact kind
of surprise that started this discussion:  use after free.

-chris


More information about the gnucash-devel mailing list