Re: [gtk-list] multiple vs. single destroy events or finalize emission (was: Re: marius ref counting stuff)

[Owen Taylor <owt1 cornell edu>]

Tim Janik <timj@gimp.org> writes:

> > 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.

OK, then your own your own for updating the REFCOUNTING file.

> > > (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.

No no no. There needs to be a concept of a widget being destroyed
that corresponds to the widgets window being destroyed on screen -
because that is something that may not correspond to any other
occurence, and it is something that applications care about.

(Once your dialog has been destroyed, you had better not do anything
more with it.)

And there needs to be a separate notion of finalization, because 
various entities hold references to a widget, so memory cannot
be destroyed when on-screen destruction happens.

> > 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.

When I asked Marius about this earlier, he said that signals
would be removed when the object was destroyed, and looking
at gtk_object_destroy that is in fact the case.

So only the default destroy handlers need to be able to be
called multiple times.

> [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]

I think Marius's idea of not making finalize a signal is that
it is very easy to do things in a "finalize" handler that really
screw up  language binding. Plus, "finalize"
doesn't occur until after everybody has dropped all references
to the widget.

(But see the weakrefs facility that was added which allows signal-like
callbacks on final destruction for people who want to keep weak
references - i.e., references that don't hold a reference count and
that become invalide when the object is destroyed)

I'm not completely sure we gain a whole lot this way, but it
does make the point clear that finalization is a very special
sort of occurence. And it also allows us to say that _no signals_
occur after a widget is destroyed.

> > 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!

This might be worthwhile to improve performance, to allow sanity
checking in various spots:

  g_return_if_fail (!GTK_WIDGET_IS_DESTROYED (widget));

(But there a million places for that... and the existing checks
for REALIZED should handle most of the important ones)

And (most importantly) to simplify and fool-proof the widget-writing
process. I think it is a good idea.
 
> > > > > - 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?

"don't see" - had to get my typo in to match yours ;-)

[ ... ]
 
> > > 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 ;)

Unless the user spends the hour flipping from one month to the
next and the calender becomes the 20Meg calender program from Hell.
We should at least make it easy for those who want to get
memory management right to get it right. But you're probably
right that no user callbacks should be made after gtk_exit ()
- it would be pretty easy to get things messed up that way.

                                        Owen