GNU bug report logs - #75636
GTK memory leaks

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 75636 <at> debbugs.gnu.org
Subject: bug#75636: GTK memory leaks
Date: Wed, 22 Jan 2025 01:06:49 +0000

"Po Lu" <luangruo <at> yahoo.com> writes:

>> commit 5ed4aaa986dbf75f8565f4d8cdd3d1d1b38b6142 (HEAD)
>> Author: Pip Cet <pipcet <at> protonmail.com>
>>
>>     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
>
> LGTM.

Thanks, pushed.

Just to clarify why I suspect this is more important than it looks: on
the feature/igc branch, a leak like this means the frame and all objects
referenced by it will become unfreeable.  The problem is not the GTK
memory usage, it's that MPS has no way of indicating "if this object is
still alive, don't move it" without simultaneously keeping it alive.

(At some point we might allow the frame to be moved even though GTK owns
a pointer to a pointer to it.  After the merge...)

>> 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;
>
> 0 as the rhs of an assignment to a pointer variable is taken for a NULL
> pointer, so there's no issue with the existing code.  Indeed on many
> systems NULL is simply defined to 0.

The issue with the old code (I've pushed this part by now) was the
duplicated assignment.  I think compilers will start to warn about that
much sooner than about the correct (if very, very slightly deprecated)
usage of plain 0 here.

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.