GNU bug report logs - #72692
Emacs 31.05 (40eecd594ac) get SIGSEGV on Linux (Linux 6.6.45 Kde Wayland)

Previous Next

Full log

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

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: execvy <at> gmail.com, 72692 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#72692: Emacs 31.05 (40eecd594ac) get SIGSEGV on Linux (Linux
 6.6.45 Kde Wayland)
Date: Mon, 26 Aug 2024 19:20:59 +0000

"Pip Cet" <pipcet <at> protonmail.com> writes:

> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>
>>> Date: Mon, 26 Aug 2024 05:52:11 +0000
>>> From: Pip Cet <pipcet <at> protonmail.com>
>>> Cc: Juri Linkov <juri <at> linkov.net>, execvy <at> gmail.com, 72692 <at> debbugs.gnu.org
>>>
>>> >> -      f->face_change = true;
>>> >
>>> > Sorry about that.  I've now reverted that change; we will have to find
>>> > a cheaper solution for the original problem.
>>>
>>> This patch, moving down the setting of f->face_change to when we
>>> definitely know it's required, seems to avoid the
>>> always-in-the-foreground problem, but I'm not sure it fixes Juri's
>>> performance issue:
>>
>> If all we want is to set the face_change flag without causing
>> performance issues, then the patch below does it, and AFAIU should
>> prevent the segfaults in this bug (please check, as I'm unable to
>> trigger the segfaults):
>
> Will do.

That was quick.  With this patch:

diff --git a/src/fontset.c b/src/fontset.c
index 16d14669c89..41d845c9bc5 100644
--- a/src/fontset.c
+++ b/src/fontset.c
@@ -921,8 +921,6 @@ free_face_fontset (struct frame *f, struct face *face)
   eassert (! BASE_FONTSET_P (fontset));
   eassert (f == XFRAME (FONTSET_FRAME (fontset)));
   ASET (Vfontset_table, face->fontset, Qnil);
-  if (face->fontset < next_fontset_id)
-    next_fontset_id = face->fontset;
   if (! NILP (FONTSET_DEFAULT (fontset)))
     {
       int id = XFIXNUM (FONTSET_ID (FONTSET_DEFAULT (fontset)));
@@ -931,8 +929,6 @@ free_face_fontset (struct frame *f, struct face *face)
       eassert (!NILP (fontset) && ! BASE_FONTSET_P (fontset));
       eassert (f == XFRAME (FONTSET_FRAME (fontset)));
       ASET (Vfontset_table, id, Qnil);
-      if (id < next_fontset_id)
-	next_fontset_id = face->fontset;
     }
   face->fontset = -1;
 }
diff --git a/src/xfaces.c b/src/xfaces.c
index 684b6ccfac7..02fc934ee4d 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -733,9 +733,12 @@ recompute_basic_faces (struct frame *f)
 {
   if (FRAME_FACE_CACHE (f))
     {
+      bool was_empty = FRAME_FACE_CACHE (f)->used == 0;
       clear_face_cache (false);
       if (!realize_basic_faces (f))
 	emacs_abort ();
+      if (!was_empty)
+	f->face_change = true;
     }
 }
 

and this hibiscus.el:

(while t
  (insert (concat (make-string 1 (floor (random 132000)))))
  (set-frame-parameter nil 'alpha-background 1.0)
  (sit-for 1.0))

emacs -Q --load hibiscus.el works.  Without the last hunk, it segfaults
almost immediately.

And the patch makes sense, too: if the face cache was empty, there's no
need to set f->face_change to true.  Is that case sufficiently common to
result in acceptable performance?

Pip

Previous Next

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