[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Patches for the sheet branch
From: |
Ben Pfaff |
Subject: |
Re: Patches for the sheet branch |
Date: |
Sat, 07 Jul 2012 22:00:40 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) |
John Darrington <address@hidden> writes:
> On Sat, Jul 07, 2012 at 11:13:49AM -0700, Ben Pfaff wrote:
>
> Do you have a rule of thumb that explains what should go in
> destroy, dispose, and finalize? The difference between destroy
> and dispose seems especially difficult to grasp.
>
> Destroy is a method of GtkObject which has been deprecated in Gtk2 for
> some time, and is absent from Gtk3. So destroy should not be used in
> new code.
>
> According to the Gtk docs, dispose should do nothing except unref objects.
> Unrealize should of course undo everthing that was done by realize. It
> should also realease any Gdk resources (such as colours etc). Finaliue
> shoud not do any unreffing.
>
> In my experience, for GtkWidgets, finalise is not realy necessary since
> everything can be done in unrealise. GObjects, which are not GtkWidgets,
> however may need finalise to actually free memory etc. Note that Gtk
> engine guarantees that it will call no signal or method on your object
> between the dispose and finalise invocations. However that guarantee is
> void if a dependent object erroneously calls such a method (either
> directly or indirectly) in its dispose method.
>
So is this an accurate summary of your take?
- Don't implement GtkObject.destroy in new code.
- Implement unrealize to undo everything done by realize and
release Gdk resources.
- Implement dispose to just unref objects.
- Implement finalize to do other cleanup.
> Some of the commits adjust refcounting of the UI manager. The
> commit messages mention that this fixes problems but don't
> explain further. Can you say more?
>
> I changed the var_sheet and data_sheet to keep a reference to the uim
> objects which they return from their _get_ui_manager objects. One
> of the comments in the code (which I removed in another commit), mentioned
> that there were problems when the final reference to uim was unreffed.
> This change fixed that problem, by ensureing that it wasn't finalised,
> until the object which owned it was disposed. In fact, I think that a
> later commit, made this change unecessary - at least it should be, once
> all the dispose/finalise methods are in order.. But it can't to a lot of
> harm keeping this extra ref.
OK. That makes enough sense to me.
Would you mind organizing the commits or editing the commit
messages to better explain the above?
> The patch "Reduce the flicker when redrawing the toolbar and
> menubar." appears to leak a reference in
> psppire_data_editor_split_window(). At least, I see nothing that
> will eventually unref de->old_vbox_widget.
>
> I think you're right. But when I add a call to _unref in dipose, it
> provokes a Critical. This needs more investigation. I think the problem
> is that make_{single,split}_datasheet returns a floating object. They
> should probably call g_object_ref_sink.
OK. Can you add a comment in psppire_data_editor_split_window()
mentioning the leak? Otherwise we'll forget about it.
> P.S. "git am" told me that some of the patches add trailing
> whitespace, not that that's really a big deal.
>
> We should probably run a sed script to remove all such whitespace.
I don't usually see the point in bothering with that, but I won't
object if you want to do it.
Thanks,
Ben.