bug#72692: Emacs 31.05 (40eecd594ac) get SIGSEGV on Linux (Linux 6.6.45 Kde Wayland)

[Pip Cet via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> From: Juri Linkov <juri@linkov.net>
>> Cc: Pip Cet <pipcet@protonmail.com>,  execvy@gmail.com,  72692@debbugs.gnu.org
>> Date: Sun, 25 Aug 2024 20:58:40 +0300
>>
>> Unfortunately, after the release branch was merged to master now,
>> it's impossible to use Emacs with this change because point motion
>> is too jumpy and not as smooth as it was before.
>> Reverting this line fixes the problem:
>>
>> -      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:

diff --git a/src/fontset.c b/src/fontset.c
index 16d14669c89..0fe65e9b481 100644
--- a/src/fontset.c
+++ b/src/fontset.c
@@ -920,6 +920,10 @@ free_face_fontset (struct frame *f, struct face *face)
     return;
   eassert (! BASE_FONTSET_P (fontset));
   eassert (f == XFRAME (FONTSET_FRAME (fontset)));
+  /* Force complete face recalculation next time we use the display
+     code, because there might be non-ASCII faces sharing this
+     fontset.  */
+  f->face_change = true;
   ASET (Vfontset_table, face->fontset, Qnil);
   if (face->fontset < next_fontset_id)
     next_fontset_id = face->fontset;
diff --git a/src/xdisp.c b/src/xdisp.c
index f9a10267bad..977495c2802 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -3227,13 +3227,13 @@ init_iterator (struct it *it, struct window *w,
       if (face_change)
        {
          face_change = false;
-         XFRAME (w->frame)->face_change = 0;
          free_all_realized_faces (Qnil);
+         XFRAME (w->frame)->face_change = 0;
        }
       else if (XFRAME (w->frame)->face_change)
        {
-         XFRAME (w->frame)->face_change = 0;
          free_all_realized_faces (w->frame);
+         XFRAME (w->frame)->face_change = 0;
        }
     }
 

However, I would still prefer a solution which doesn't, even for a short
period of time, leave the non-ASCII faces' ->ascii_face pointing to a
face which has been freed, and their ->fontset identifying a fontset
which might be nil.

Is it true that all non-ASCII faces share their ASCII face's hash and
thus live in the same collision chain?  We could walk that and unrealize
them all when the ASCII face is unrealized, then.

Something like this:

diff --git a/src/xfaces.c b/src/xfaces.c
index 684b6ccfac7..bfc6b76d58a 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4585,6 +4585,8 @@ make_realized_face (Lisp_Object *attr)
 }
 
 
+static void uncache_face (struct face_cache *c, struct face *face);
+
 /* Free realized face FACE, including its X resources.  FACE may
    be null.  */
 
@@ -4596,9 +4598,28 @@ free_realized_face (struct frame *f, struct face *face)
 #ifdef HAVE_WINDOW_SYSTEM
       if (FRAME_WINDOW_P (f))
 	{
-	  /* Free fontset of FACE if it is ASCII face.  */
-	  if (face->fontset >= 0 && face == face->ascii_face)
-	    free_face_fontset (f, face);
+	  if (face == face->ascii_face)
+	    {
+	      struct face *next;
+	      /* Because cache_face puts non-ASCII faces last in the
+		 collision chain, we only have to check in one
+		 direction.  */
+	      for (struct face *face_nonascii = face->next; face_nonascii;
+		   face_nonascii = next)
+		{
+		  next = face_nonascii->next;
+
+		  if (face == face_nonascii->ascii_face)
+		    {
+		      uncache_face (FRAME_FACE_CACHE (f), face_nonascii);
+		      free_realized_face (f, face_nonascii);
+		    }
+		}
+
+	      /* Free fontset of FACE if it is ASCII face.  */
+	      if (face->fontset >= 0)
+		free_face_fontset (f, face);
+	    }
 
 #ifdef HAVE_X_WINDOWS
 	  /* This function might be called with the frame's display



Pip