multiple vs. single destroy events or finalize emission (was: Re: marius ref counting stuff)

[Tim Janik <Tim Janik Hamburg Netsurf DE>]

hey guys,

sorry for the quoting, but i'd rather like to see this subject discussed in
public as well (this has been on the gtk cvs-developers list).

On Sat, 31 Jan 1998, Tim Janik wrote:
> On Fri, 30 Jan 1998, Owen Taylor wrote:
> 
> > 
> > Tim Janik <timj@gimp.org> writes:
> > 
> > > On Fri, 30 Jan 1998, Owen Taylor wrote:
> > > 
> > > > 
> > > > Time Janik <timj@gimp.org>
> > > > 
> > > > - The following text
> > > > 
> > > > + Toplevel widgets, which don't have a `natural' parent, are adopted by
> > > > + a special widget, maybe a GtkDisplay or GtkScreen.  This special
> > > > + parent of all toplevel widgets is never freed.  The toplevel widgets
> > > > + are added to this parent as soon as they are created. [Maybe this
> > > > + special widget will only exist conceptually because toplevel widgets
> > > > + are identified by parent == NULL through-out the code.]
> > > > + 
> > > > 
> > > > Should be changed to say that the toplevel widget's parent _is_
> > > > only conceptual. (I don't see a reason to add a real parent - 
> > > > what is _its_ parent then?)
> > > 
> > > feel free to do so...
> > 
> > Once I figure out the GTK_TOPLEVEL[_ONSCREEN] stuff, I'll do
> > something about it.
> 
> hm, don't know wether i postedt this puplically in english or to marius
> only - wait...
> 
> a, got it in my TODO list:
> * hm, seems like GTK_TOPLEVEL_ONSCREEN isn't justified...
> 
> since GTK_TOPLEVEL_ONSCREEN is used only once in gtkwidget.c and just flags
> an adiitional reference count to toplevel widgets, i really doubt it's
> justified. i have to have a look at *when* gtk_widget_set_parent(, NULL) is
> called, but once we got that worked out, i think we can skip this flag.
> 
> > > > - See above on the ltconfig change
> > > > 
> > > > - What?
> > > > 
> > > >     /* get rid of all the column buttons */
> > > >     for (i = 0; i < clist->columns; i++)
> > > > !     {
> > > > !       gtk_widget_destroy (clist->column[i].button);
> > > > !       gtk_widget_unparent (clist->column[i].button);
> > > > !     }
> > > 
> > > clist is still to be worked on.
> > > 
> > > > Shouldn't destroying a widget automatically unparent it?
> > > > This is going to break a lot of code if it doesn't
> > > 
> > > i wondered the same (at the first glance), but with the emission
> > > of the destroy signal, a widget will be removed from it's parent
> > > if needed (container_remove in widget_real_destroy).
> > 
> > I think I've finished this now. It turns out that CLists are
> > a rather special case. Since they don't have a _remove()
> > function, their children won't be automatically destroyed
> > when they are removed.
> > 
> > I ended up just doing
> > 
> >       gtk_widget_unparent (clist->column[i].button);
> > 
> > And not the destroy since the destroy will be triggered anyways
> > when the refcount drops to zero.
> > 
> > (This is actually a common problem. Right now, most "destroy"
> > functions are being called twice, once when they are explicitely
> > destroyed and once when the refcount drops to zero. This isn't
> > actually a bug - "destroy" functions are supposed to be 
> > able to be called multiple times - but is certainly isn't
> > efficient.
> 
> and, this is one major problem.
> i asked marius about the portion in gtklist.c which i took as an
> exapmle (haven't received an answer yet):
> 
>   for (node = list->children; node; node = node->next)
>     {
>       GtkWidget *child;
> 
>       child = (GtkWidget *)node->data;
>       gtk_widget_ref (child);
>       gtk_widget_unparent (child);
>       gtk_widget_destroy (child);
>       gtk_widget_unref (child);
>     }
>   g_list_free (list->children);
>   list->children = NULL;
> 
> considering the fact that gtk_object_unref will emit the destroy signal
> for gtkobjects once the reference count dropped to zero, i don't actually
> see in a point to keep gtk_widget_destroy in the current manner (i.e.

[ see a point in keeping gtk_widget_destroy in the current manner (i.e. ]

> calling gtk_object_destroy). since we know the reference count will
> drop to zero *somewhen* and therefore the "destroy" signal will be emitted,
> gtk_widget_destroy should just be a matter of doing gtk_object_unref,
> and gtk_widget_destroy shouldn't be used through out the gtk code anymore.
> i think marius made a mistake in letting it still do the emission.
> 
> conceptually the "destroy" signal was what now is the "finalize" signal.
> applications use it for e.g.
              [used]
> gtk_signal_connect_object (GTK_OBJECT (window), "destroy",
> 			   (GtkSignalFunc*) g_free,
> 			   (GtkObject*) my_g_malloced_array_pointer);
> this can now be emitted twice and *will* cause bad actions.

[excerpt from the new REFCOUNTING file:
 ...Every widget must be revised to have a
 proper "destroy" function, etc.  Such a destroy function must be able
 to be called any number of times and generally leave the widget in a
 minimal but consistent state.  The "finalization" function is new and
 should perform last-minute cleanup actions.  It can assume that the
 "destroy" function has been called as the last function on this
 widget.]

i *don't* like the idea of multiple destruction calls!
once i "render a widget unusable", i don't see a point in rendering
it again "unusable". (is it the "unusablerer" or "more unusable"? ;)

> i'd rather see us asuring that the destroy emission is done only *once*
> than changing any occourance of "destroy" and .*signal::destroy" to
> "finalize" and .*signal::finalize" through out the globe.

[if we make GtkObject::finalize actually *emit* a signal]

> even if an application didn't freak around with the callback function
> as it's done above with `g_free', you have been able to savely
> rely on the fact that it's save to not care about reinvokation of
> destroy handlers (this just makes the above point stronger).

on this special issue, i'm even willing to implement a new
object flag GTK_IS_DESTROYED which will prevent subsequent emissions!

> > 
> > > > - Setting pointers to NULL in a finalize procedure strikes
> > > >   me as excessive conservatism... (but, hey, it doesn't
> > > >   hurt)
> > > 
> > > this was mainly done in *_finalize functions.

[ uhm, sorry i meant to say: this was mainly done in *_destroy functions]

> > 
> > ? It turns out this is important in destroy functions since
> > they have to be able to be called multiple times, but I still
> > the point for *_finalize functions.
> 
> you still [what] the point for _finalize functions?
> 
> > > > - The changes to testinput.c disturb me a bit. Calling
> > > >   gtk_exit () really should destroy all windows itself;
> > > >   if that is possible. (I suppose that would, at least,
> > > >   mean keeping a list of all toplevel windows)
> > > 
> > > i only applied the change for testgtk.
> > > gtk_exit() shouldn't call the appropriate desctructors for
> > > toplevels imho.
> > > this can be done in main() after the gtk_main() call.
> > 
> > Hmm. I guess you are saying that we should be encouragin
> > people to call gtk_main_quit() instead of gtk_exit().
> 
> for the ones that care about proper destruction.
> for the ones who don't, we should leave gtk_exit() as
> not invoking object destroy functions, otherwise you'll
> have to track down stuff like
> void my_button_clicked_func (GtkWidget *widget, gpointer func_data)
> {
>   gtk_exit (0);
>   /* dunno why, but i still have segfaults in my application after
>      i called exit. i therefore consider this toolkit buggy and dump.
>      - Mr. Dummy
>      */
> }
> which will be hard to justify. ;))

[ hey, i did *not* mean to be impolity by calling joe user "Mr. Dummy" ;) ]

> > In some cases, it may be easier for an application not to
> > keep track of all its toplevels (at least in one place).
> 
> those can then ignore living objects as well...
> actually caring about living objects is most important
> in language bindings, but extremely less in
> MyCalendarAppWithAnAverageRuntimeOfOneHour ;)
> 
> > But in general, it isn't a tragedy if all the windows don't
> > get destroyed. (Unless you are trying to catch leaks
> > with Purify or some such tool).
> 
> on a side note, i'll have to visit my parents now, but once i've
> done that, i'll create a mega patch from what i've done yesterday,
> read through it again, and write up a mega changelog entry ;)
> 

ok, ok, for those of you who are still with me, i admitt i still have to
do that since i got back to late... ;)

> > 
> > Regards,
> >                                         Owen
> > 
> 
> ---
> ciaoTJ
> 
> 

---
ciaoTJ