GNU bug report logs - #77128
29.4; Memory leak in visual bell (pgtk_flash) on pure GTK Wayland builds

Previous Next

To reply to this bug, email your comments to 77128 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Alessandro Di Marco <dmr <at> ethzero.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.4;
 Memory leak in visual bell (pgtk_flash) on pure GTK Wayland builds
Date: Wed, 19 Mar 2025 22:45:12 +0100

[Message part 1 (text/plain, inline)]

From: dmr <at> oya.mail-host-address-is-not-set
To: bug-gnu-emacs <at> gnu.org
Subject: 29.4; Memory leak in visual bell (pgtk_flash) on pure GTK Wayland
builds
--text follows this line--
When Emacs is built with “pure GTK” internals for full Wayland
compatibility, repeated invocations of the visual bell (triggered via
pgtk_flash) result in a significant memory leak. The issue occurs
because pgtk_flash creates a new Cairo surface (cr_surface_visible_bell)
using cairo_surface_create_similar each time the visual bell is
activated. If flashes occur in rapid succession, a previously created
surface may still be stored when a new flash is initiated. Although a
timer (atimer_visible_bell) is set to call recover_from_visible_bell
after 50 ms (which in turn destroys the surface), the code does not
check whether an existing surface and its timer are active before
overwriting them.

This leads to the accumulation of unreleased surfaces if the visual bell
is triggered faster than the timer can clear them. To address this, the
patch attached here explicitly checks if cr_surface_visible_bell is
non‑NULL and, if so, calls cairo_surface_destroy to free it and cancels
any active atimer before assigning a new surface.

Signed-off-by: Alessandro Di Marco <dmr <at> ethzero.com>
---
diff '--color=auto' -urN old/src/pgtkterm.c new/src/pgtkterm.c
--- old/src/pgtkterm.c 2025-03-19 09:46:10.652352642 +0100
+++ new/src/pgtkterm.c 2025-03-19 20:54:32.448191644 +0100
@@ -3782,6 +3782,18 @@
       cairo_fill (cr);
     }

+  if (FRAME_X_OUTPUT (f)->cr_surface_visible_bell != NULL)
+    {
+      cairo_surface_destroy (FRAME_X_OUTPUT (f)->cr_surface_visible_bell);
+      FRAME_X_OUTPUT (f)->cr_surface_visible_bell = NULL;
+    }
+
+  if (FRAME_X_OUTPUT (f)->atimer_visible_bell != NULL)
+    {
+      cancel_atimer (FRAME_X_OUTPUT (f)->atimer_visible_bell);
+      FRAME_X_OUTPUT (f)->atimer_visible_bell = NULL;
+    }
+
   FRAME_X_OUTPUT (f)->cr_surface_visible_bell = surface;

   delay = make_timespec (0, 50 * 1000 * 1000);
--

Rationale: The patch ensures that if a previous flash’s surface (and
timer) exists, it is cleaned up before a new flash surface is
allocated. This prevents the accumulation of stale surfaces and
eliminates the memory leak observed on repeated visual bell invocations.

In GNU Emacs 29.4 (build 1, aarch64-unknown-linux-gnu, GTK+ Version
 3.24.42, cairo version 1.18.2) of 2025-03-19 built on localhost
System Description: Gentoo Linux

Configured using:
 'configure --prefix=/usr --build=aarch64-unknown-linux-gnu
 --host=aarch64-unknown-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --datarootdir=/usr/share
 --disable-silent-rules --docdir=/usr/share/doc/emacs-29.4-r2
 --htmldir=/usr/share/doc/emacs-29.4-r2/html --libdir=/usr/lib64
 --program-suffix=-emacs-29 --includedir=/usr/include/emacs-29
 --infodir=/usr/share/info/emacs-29 --localstatedir=/var
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --without-compress-install --without-hesiod --without-pop
 --with-file-notification=inotify --with-pdumper --enable-acl
 --with-dbus --without-modules --without-gameuser --with-libgmp
 --without-gpm --without-native-compilation --without-json
 --without-kerberos --without-kerberos5 --without-lcms2 --without-xml2
 --without-mailutils --without-selinux --without-sqlite3 --with-gnutls
 --without-libsystemd --with-threads --without-tree-sitter
 --without-wide-int --with-sound=no --with-zlib --with-pgtk --without-x
 --without-ns --with-toolkit-scroll-bars --without-gconf
 --without-xwidgets --without-gsettings --with-harfbuzz --without-libotf
 --without-m17n-flt --without-gif --with-jpeg --without-png
 --without-rsvg --without-tiff --without-webp --without-imagemagick
 --with-dumping=pdumper
 'CFLAGS=-march=armv8.6-a+simd+fp16+crypto+i8mm+bf16 -mtune=native -O2
 -pipe -fno-fast-math -ffp-contract=off' CPPFLAGS= 'LDFLAGS=-Wl,-O1
 -Wl,--as-needed''

Configured features:
ACL CAIRO DBUS FREETYPE GLIB GMP GNUTLS HARFBUZZ JPEG NOTIFY INOTIFY
PDUMPER PGTK SECCOMP THREADS TOOLKIT_SCROLL_BARS XIM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Diff

Minor modes in effect:
  server-mode: t
  display-time-mode: t
  which-function-mode: t
  save-place-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
~/.emacs.d/lisp/backup-each-save hides
/usr/share/emacs/site-lisp/backup-each-save/backup-each-save

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny rfc822
mml mml-sec epa derived epg rfc6068 epg-config gnus-util
text-property-search time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils smerge-mode diff diff-mode
easy-mmode dired-aux dired dired-loaddefs add-log hideshow server rx
time cus-load which-func imenu auth-source cl-seq eieio eieio-core
cl-macs cl-loaddefs cl-lib password-cache json subr-x map byte-opt gv
bytecomp byte-compile ansi-color flyspell ispell saveplace site-gentoo
rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win
term/common-win pgtk-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify dynamic-setting font-render-setting cairo gtk
pgtk multi-tty make-network-process emacs)

Memory information:
((conses 16 75274 8149)
 (symbols 48 8892 0)
 (strings 32 25562 2803)
 (string-bytes 1 763362)
 (vectors 16 15529)
 (vector-slots 8 216819 8335)
 (floats 8 35 33)
 (intervals 56 565 0)
 (buffers 976 14))

[Message part 2 (text/html, inline)]

Message #8 received at 77128 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alessandro Di Marco <dmr <at> ethzero.com>, Po Lu <luangruo <at> yahoo.com>
Cc: 77128 <at> debbugs.gnu.org
Subject: Re: bug#77128: 29.4;
 Memory leak in visual bell (pgtk_flash) on pure GTK Wayland builds
Date: Thu, 20 Mar 2025 09:52:40 +0200

> From: Alessandro Di Marco <dmr <at> ethzero.com>
> Date: Wed, 19 Mar 2025 22:45:12 +0100
> 
> When Emacs is built with “pure GTK” internals for full Wayland
> compatibility, repeated invocations of the visual bell (triggered via
> pgtk_flash) result in a significant memory leak. The issue occurs
> because pgtk_flash creates a new Cairo surface (cr_surface_visible_bell)
> using cairo_surface_create_similar each time the visual bell is
> activated. If flashes occur in rapid succession, a previously created
> surface may still be stored when a new flash is initiated. Although a
> timer (atimer_visible_bell) is set to call recover_from_visible_bell
> after 50 ms (which in turn destroys the surface), the code does not
> check whether an existing surface and its timer are active before
> overwriting them.
> 
> This leads to the accumulation of unreleased surfaces if the visual bell
> is triggered faster than the timer can clear them. To address this, the
> patch attached here explicitly checks if cr_surface_visible_bell is
> non‑NULL and, if so, calls cairo_surface_destroy to free it and cancels
> any active atimer before assigning a new surface.
> 
> Signed-off-by: Alessandro Di Marco <dmr <at> ethzero.com>
> ---
> diff '--color=auto' -urN old/src/pgtkterm.c new/src/pgtkterm.c
> --- old/src/pgtkterm.c 2025-03-19 09:46:10.652352642 +0100
> +++ new/src/pgtkterm.c 2025-03-19 20:54:32.448191644 +0100
> @@ -3782,6 +3782,18 @@
>        cairo_fill (cr);
>      }
>  
> +  if (FRAME_X_OUTPUT (f)->cr_surface_visible_bell != NULL)
> +    {
> +      cairo_surface_destroy (FRAME_X_OUTPUT (f)->cr_surface_visible_bell);
> +      FRAME_X_OUTPUT (f)->cr_surface_visible_bell = NULL;
> +    }
> +
> +  if (FRAME_X_OUTPUT (f)->atimer_visible_bell != NULL)
> +    {
> +      cancel_atimer (FRAME_X_OUTPUT (f)->atimer_visible_bell);
> +      FRAME_X_OUTPUT (f)->atimer_visible_bell = NULL;
> +    }
> +  
>    FRAME_X_OUTPUT (f)->cr_surface_visible_bell = surface;
>  
>    delay = make_timespec (0, 50 * 1000 * 1000);
> --
> 
> Rationale: The patch ensures that if a previous flash’s surface (and
> timer) exists, it is cleaned up before a new flash surface is
> allocated. This prevents the accumulation of stale surfaces and
> eliminates the memory leak observed on repeated visual bell invocations.

Thanks.

Po Lu, any comments?  If you agree with the patch, do you think it's
safe enough to go into Emacs 30.2?

Message #11 received at 77128 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 77128 <at> debbugs.gnu.org, Alessandro Di Marco <dmr <at> ethzero.com>
Subject: Re: bug#77128: 29.4; Memory leak in visual bell (pgtk_flash) on
 pure GTK Wayland builds
Date: Thu, 20 Mar 2025 16:21:32 +0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Alessandro Di Marco <dmr <at> ethzero.com>
>> Date: Wed, 19 Mar 2025 22:45:12 +0100
>> 
>> When Emacs is built with “pure GTK” internals for full Wayland
>> compatibility, repeated invocations of the visual bell (triggered via
>> pgtk_flash) result in a significant memory leak. The issue occurs
>> because pgtk_flash creates a new Cairo surface (cr_surface_visible_bell)
>> using cairo_surface_create_similar each time the visual bell is
>> activated. If flashes occur in rapid succession, a previously created
>> surface may still be stored when a new flash is initiated. Although a
>> timer (atimer_visible_bell) is set to call recover_from_visible_bell
>> after 50 ms (which in turn destroys the surface), the code does not
>> check whether an existing surface and its timer are active before
>> overwriting them.
>> 
>> This leads to the accumulation of unreleased surfaces if the visual bell
>> is triggered faster than the timer can clear them. To address this, the
>> patch attached here explicitly checks if cr_surface_visible_bell is
>> non‑NULL and, if so, calls cairo_surface_destroy to free it and cancels
>> any active atimer before assigning a new surface.
>> 
>> Signed-off-by: Alessandro Di Marco <dmr <at> ethzero.com>
>> ---
>> diff '--color=auto' -urN old/src/pgtkterm.c new/src/pgtkterm.c
>> --- old/src/pgtkterm.c 2025-03-19 09:46:10.652352642 +0100
>> +++ new/src/pgtkterm.c 2025-03-19 20:54:32.448191644 +0100
>> @@ -3782,6 +3782,18 @@
>>        cairo_fill (cr);
>>      }
>>  
>> +  if (FRAME_X_OUTPUT (f)->cr_surface_visible_bell != NULL)
>> +    {
>> +      cairo_surface_destroy (FRAME_X_OUTPUT (f)->cr_surface_visible_bell);
>> +      FRAME_X_OUTPUT (f)->cr_surface_visible_bell = NULL;
>> +    }
>> +
>> +  if (FRAME_X_OUTPUT (f)->atimer_visible_bell != NULL)
>> +    {
>> +      cancel_atimer (FRAME_X_OUTPUT (f)->atimer_visible_bell);
>> +      FRAME_X_OUTPUT (f)->atimer_visible_bell = NULL;
>> +    }
>> +  
>>    FRAME_X_OUTPUT (f)->cr_surface_visible_bell = surface;
>>  
>>    delay = make_timespec (0, 50 * 1000 * 1000);
>> --
>> 
>> Rationale: The patch ensures that if a previous flash’s surface (and
>> timer) exists, it is cleaned up before a new flash surface is
>> allocated. This prevents the accumulation of stale surfaces and
>> eliminates the memory leak observed on repeated visual bell invocations.
>
> Thanks.
>
> Po Lu, any comments?  If you agree with the patch, do you think it's
> safe enough to go into Emacs 30.2?

I believe that it is safe enough, and also that the leak impacts the
X11/Cairo configuration.  I'll install this change in both
configurations this weekend.

Message #14 received at 77128 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: 77128 <at> debbugs.gnu.org, dmr <at> ethzero.com
Subject: Re: bug#77128: 29.4; Memory leak in visual bell (pgtk_flash) on
 pure GTK Wayland builds
Date: Sat, 29 Mar 2025 14:35:14 +0300

> From: Po Lu <luangruo <at> yahoo.com>
> Cc: Alessandro Di Marco <dmr <at> ethzero.com>,  77128 <at> debbugs.gnu.org
> Date: Thu, 20 Mar 2025 16:21:32 +0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Alessandro Di Marco <dmr <at> ethzero.com>
> >> Date: Wed, 19 Mar 2025 22:45:12 +0100
> >> 
> >> When Emacs is built with “pure GTK” internals for full Wayland
> >> compatibility, repeated invocations of the visual bell (triggered via
> >> pgtk_flash) result in a significant memory leak. The issue occurs
> >> because pgtk_flash creates a new Cairo surface (cr_surface_visible_bell)
> >> using cairo_surface_create_similar each time the visual bell is
> >> activated. If flashes occur in rapid succession, a previously created
> >> surface may still be stored when a new flash is initiated. Although a
> >> timer (atimer_visible_bell) is set to call recover_from_visible_bell
> >> after 50 ms (which in turn destroys the surface), the code does not
> >> check whether an existing surface and its timer are active before
> >> overwriting them.
> >> 
> >> This leads to the accumulation of unreleased surfaces if the visual bell
> >> is triggered faster than the timer can clear them. To address this, the
> >> patch attached here explicitly checks if cr_surface_visible_bell is
> >> non‑NULL and, if so, calls cairo_surface_destroy to free it and cancels
> >> any active atimer before assigning a new surface.
> >> 
> >> Signed-off-by: Alessandro Di Marco <dmr <at> ethzero.com>
> >> ---
> >> diff '--color=auto' -urN old/src/pgtkterm.c new/src/pgtkterm.c
> >> --- old/src/pgtkterm.c 2025-03-19 09:46:10.652352642 +0100
> >> +++ new/src/pgtkterm.c 2025-03-19 20:54:32.448191644 +0100
> >> @@ -3782,6 +3782,18 @@
> >>        cairo_fill (cr);
> >>      }
> >>  
> >> +  if (FRAME_X_OUTPUT (f)->cr_surface_visible_bell != NULL)
> >> +    {
> >> +      cairo_surface_destroy (FRAME_X_OUTPUT (f)->cr_surface_visible_bell);
> >> +      FRAME_X_OUTPUT (f)->cr_surface_visible_bell = NULL;
> >> +    }
> >> +
> >> +  if (FRAME_X_OUTPUT (f)->atimer_visible_bell != NULL)
> >> +    {
> >> +      cancel_atimer (FRAME_X_OUTPUT (f)->atimer_visible_bell);
> >> +      FRAME_X_OUTPUT (f)->atimer_visible_bell = NULL;
> >> +    }
> >> +  
> >>    FRAME_X_OUTPUT (f)->cr_surface_visible_bell = surface;
> >>  
> >>    delay = make_timespec (0, 50 * 1000 * 1000);
> >> --
> >> 
> >> Rationale: The patch ensures that if a previous flash’s surface (and
> >> timer) exists, it is cleaned up before a new flash surface is
> >> allocated. This prevents the accumulation of stale surfaces and
> >> eliminates the memory leak observed on repeated visual bell invocations.
> >
> > Thanks.
> >
> > Po Lu, any comments?  If you agree with the patch, do you think it's
> > safe enough to go into Emacs 30.2?
> 
> I believe that it is safe enough, and also that the leak impacts the
> X11/Cairo configuration.  I'll install this change in both
> configurations this weekend.

Did you have time to install it?  When you do, please close the bug.

Previous Next

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