Subject: bug#75636: GTK memory leaks

From: Pip Cet <[email-masked]>
Date: Tue, 21 Jan 2025 11:24:21 +0000

"Po Lu" <[email-masked]> writes:

> Pip Cet <[email-masked]> writes:
>
>> "Po Lu" <[email-masked]> writes:
>>
>>> Pip Cet <[email-masked]> writes:
>>>
>>>> Creating and destroying GTK frames (using gtkutil.c, not PGTK) currently
>>>> leaks some memory, particularly when using tool bars.
>>>>
>>>> 1. The input "multicontext" created by:
>>>>
>>>>   imc = gtk_im_multicontext_new ();
>>>>   g_object_ref (imc);
>>>>
>>>> is only unref'd once when destroying the frame.  This leaves the
>>>> refcount at 1 and leaks the object.
>>>
>>> Strange, I was under the impression that IM contexts descended from
>>> GInitiallyUnowned.  Evidently they don't, and as such, removing the
>>> g_object_ref will suffice.
>>
>> Thanks!  I'll just do that, then?  I think it's okay only to do it on
>> master, as it's a minor bug in the !HAVE_MPS case.
>
> Sounds good to me.
>
>>>> 2. free_frame_tool_bar is never called in my experiments, so the toolbar
>>>> widget isn't destroyed.
>>>
>>> It is only supposed to be called when a frame's tool bar is disabled.
>>> while it is x_free_frame_resources that should release all resources
>>> when a frame is to be destroyed.
>>
>> Thanks!  I'll come up with a patch and discuss it further for this one,
>> because it's not quite trivial.
>>
>> Pip
>
> Thanks.

This showed up during testing, but might ultimately have been because I
deleted the new frame too soon.  I can only reproduce it on the
feature/igc branch, but that might be because the timing is different on
that branch.

When we create the initial tool bar, f->tool_bar_items is still Qnil.
In that case, we don't pack the toolbar widget, but create it and
connect signals to it.

xg_free_frame_widgets does not explicitly delete the toolbar, which
works only if it's packed and destroyed implicitly.

I think this is right, and the bug isn't reproducible on the master
branch only because of timing differences:

commit 5ed4aaa986dbf75f8565f4d8cdd3d1d1b38b6142 (HEAD)
Author: Pip Cet <[email-masked]>

    Destroy GTK tool bar widget if it was never attached (bug#75636)
    
    * src/gtkutil.c (xg_free_frame_widgets): Call gtk_widget_destroy on an
    unpacked toolbar widget.

diff --git a/src/gtkutil.c b/src/gtkutil.c
index 0e9dd4dfe11..97582a524da 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -1885,6 +1885,12 @@ xg_free_frame_widgets (struct frame *f)
       if (tbinfo)
         xfree (tbinfo);
 
+      if (x->toolbar_widget && !x->toolbar_is_packed)
+	{
+	  gtk_widget_destroy (x->toolbar_widget);
+	  x->toolbar_widget = NULL;
+	}
+
       /* x_free_frame_resources should have taken care of it */
 #ifndef HAVE_PGTK
 #ifdef HAVE_XDBE

I think this second patch might also avoid future compiler warnings:

diff --git a/src/gtkutil.c b/src/gtkutil.c
index 97582a524da..73e9f778c75 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -6123,8 +6123,7 @@ free_frame_tool_bar (struct frame *f)
       else
         gtk_widget_destroy (x->toolbar_widget);
 
-      x->toolbar_widget = 0;
-      x->toolbar_widget = 0;
+      x->toolbar_widget = NULL;
       x->toolbar_is_packed = false;
       FRAME_TOOLBAR_TOP_HEIGHT (f) = FRAME_TOOLBAR_BOTTOM_HEIGHT (f) = 0;
       FRAME_TOOLBAR_LEFT_WIDTH (f) = FRAME_TOOLBAR_RIGHT_WIDTH (f) = 0;

If we can apply the first, minor patch, I think this bug can be closed;
if I run into further GTK issues, I'll open a new one.

Pip