Subject: Re: bug#42943: 28.0.50; Emacsclient crashes in ftcrfont_glyph_extents

From: Robert Pluim <[email-masked]>
Date: Sat, 24 Oct 2020 17:34:47 +0200

>>>>> On Sat, 24 Oct 2020 17:12:04 +0300, Eli Zaretskii <[email-masked]> said:

    >> From: Robert Pluim <[email-masked]>
    >> Cc: [email-masked],  [email-masked],  [email-masked]
    >> Date: Sat, 24 Oct 2020 15:27:39 +0200
    >> 
    >> static int
    >> fill_gstring_glyph_string (struct glyph_string *s, int face_id,
    >> int start, int end, int overlaps)
    >> {
    >> struct glyph *glyph, *last;
    >> Lisp_Object lgstring;
    >> int i;
    >> bool glyph_not_available_p;
    >> 
    s-> for_overlaps = overlaps;
    >> glyph = s->row->glyphs[s->area] + start;
    >> last = s->row->glyphs[s->area] + end;
    >> glyph_not_available_p = glyph->glyph_not_available_p;
    s-> cmp_id = glyph->u.cmp.id;
    s-> cmp_from = glyph->slice.cmp.from;
    s-> cmp_to = glyph->slice.cmp.to + 1;
    s-> face = FACE_FROM_ID (s->f, face_id);
    >> lgstring = composition_gstring_from_id (s->cmp_id);
    s-> font = XFONT_OBJECT (LGSTRING_FONT (lgstring)); <----
    >> 
    >> so itʼs the caching in the Lisp_Object for the composition thatʼs
    >> causing the problem.

    Eli> OK, so when we are about to release a font, we need to go over all the
    Eli> LGSTRING objects in gstring_hash_table, and remove from that cache
    Eli> every LGSTRING whose LGSTRING_FONT object holds the font we are about
    Eli> to release.

I stuck an Fclrhash (gstring_hash_table) in ftcrfont_close, and it no
longer crashes (and valgrind is happy). The following also doesnʼt
crash, but I worry about the quadratic behaviour when removing the
entries from the hash_table. Do we need a
hash_remove_from_table_by_index? Am I being overly paranoid?

Iʼve not checked with other font backends, so the same treatment might
be needed there.

diff --git i/src/composite.c w/src/composite.c
index 984e0d9cda..93a700897f 100644
--- i/src/composite.c
+++ w/src/composite.c
@@ -639,6 +639,18 @@ compose_text (ptrdiff_t start, ptrdiff_t end, Lisp_Object components,
 
 static Lisp_Object gstring_lookup_cache (Lisp_Object);
 
+void
+gstring_remove_from_cache_by_font (struct font * font)
+{
+  struct Lisp_Hash_Table *h = XHASH_TABLE (gstring_hash_table);
+  for (ptrdiff_t i = 0; i < HASH_TABLE_SIZE (h); ++i)
+    {
+      Lisp_Object k = HASH_KEY (h, i);
+      if (!EQ (k, Qunbound) && font == XFONT_OBJECT (LGSTRING_FONT (HASH_VALUE (h, i))))
+          hash_remove_from_table (h, k);
+    }
+}
+
 static Lisp_Object
 gstring_lookup_cache (Lisp_Object header)
 {
diff --git i/src/composite.h w/src/composite.h
index 239f1e531e..2c30ee1933 100644
--- i/src/composite.h
+++ w/src/composite.h
@@ -331,6 +331,8 @@ #define LGLYPH_WADJUST(g) (VECTORP (LGLYPH_ADJUSTMENT (g)) \
 
 extern ptrdiff_t composition_adjust_point (ptrdiff_t, ptrdiff_t);
 
+extern void gstring_remove_from_cache_by_font (struct font *);
+
 INLINE_HEADER_END
 
 #endif /* not EMACS_COMPOSITE_H */
diff --git i/src/ftcrfont.c w/src/ftcrfont.c
index 4892a34a3a..61a2b1290f 100644
--- i/src/ftcrfont.c
+++ w/src/ftcrfont.c
@@ -283,6 +283,7 @@ ftcrfont_close (struct font *font)
   struct font_info *ftcrfont_info = (struct font_info *) font;
 
   block_input ();
+  gstring_remove_from_cache_by_font (font);
 #ifdef HAVE_LIBOTF
   if (ftcrfont_info->otf)
     {

Robert
--