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

From: Eli Zaretskii <[email-masked]>
Date: Tue, 27 Aug 2024 14:47:08 +0300

> Date: Mon, 26 Aug 2024 19:20:59 +0000
> From: Pip Cet <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked]
> 
> 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.

That was the main idea of the patch I proposed, except that avoiding
to set the face_change flag when all we have in the cache are the
basic faces is a bit stronger, no?

> Is that case sufficiently common to result in acceptable
> performance?

Which case?