bug#71929: 30.0.60; crash in mark_image_cache

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

Sean Whitton <spwhitton@spwhitton.name> writes:

> I recompiled with this.  It crashed again this morning.  Unfortunately
> it just crashed in the same way, without hitting any other failed
> assertions along the way.  Here is the backtrace.
>
>   0 in mark_image_cache of image.c:3775
>   1 in mark_frame of alloc.c:7063
>   2 in process_mark_stack of alloc.c:7303
>   3 in mark_objects of alloc.c:7512
>   4 in mark_vectorlike of alloc.c:6891
>   5 in mark_window of alloc.c:7072
>   6 in process_mark_stack of alloc.c:7307
>   7 in mark_objects of alloc.c:7512
>   8 in mark_vectorlike of alloc.c:6891
>   9 in mark_frame of alloc.c:7037
>  10 in process_mark_stack of alloc.c:7303
>  11 in mark_objects of alloc.c:7512
>  12 in mark_vectorlike of alloc.c:6891
>  13 in mark_window of alloc.c:7072
>  14 in process_mark_stack of alloc.c:7307
>  15 in mark_object of alloc.c:7504
>  16 in mark_char_table of alloc.c:6920
>  17 in mark_char_table of alloc.c:6917
>  18 in process_mark_stack of alloc.c:7341
>  19 in mark_object of alloc.c:7504
>  20 in mark_char_table of alloc.c:6920
>  21 in mark_char_table of alloc.c:6917
>  22 in process_mark_stack of alloc.c:7341
>  23 in mark_object of alloc.c:7504
>  24 in mark_interval_tree_1 of alloc.c:1529
>  25 in traverse_intervals_noorder of intervals.c:243
>  26 in traverse_intervals_noorder of intervals.c:248
>  27 in traverse_intervals_noorder of intervals.c:248
>  28 in traverse_intervals_noorder of intervals.c:248
>  29 in mark_interval_tree of alloc.c:1538
>  30 in mark_buffer of alloc.c:6958
>  31 in process_mark_stack of alloc.c:7299
>  32 in mark_objects of alloc.c:7512
>  33 in mark_vectorlike of alloc.c:6891
>  34 in mark_buffer of alloc.c:6954
>  35 in process_mark_stack of alloc.c:7299
>  36 in mark_object of alloc.c:7504
>  37 in mark_discard_killed_buffers of alloc.c:7020
>  38 in mark_window of alloc.c:7087
>  39 in process_mark_stack of alloc.c:7307
>  40 in mark_objects of alloc.c:7512
>  41 in mark_vectorlike of alloc.c:6891
>  42 in mark_frame of alloc.c:7037
>  43 in process_mark_stack of alloc.c:7303
>  44 in mark_object of alloc.c:7504
>  45 in mark_interval_tree_1 of alloc.c:1529
>  46 in traverse_intervals_noorder of intervals.c:243
>  47 in mark_interval_tree of alloc.c:1538
>  48 in process_mark_stack of alloc.c:7264
>  49 in mark_objects of alloc.c:7512
>  50 in mark_vectorlike of alloc.c:6891
>  51 in mark_buffer of alloc.c:6954
>  52 in process_mark_stack of alloc.c:7299
>  53 in mark_object of alloc.c:7504
>  54 in mark_interval_tree_1 of alloc.c:1529
>  55 in traverse_intervals_noorder of intervals.c:243
>  56 in traverse_intervals_noorder of intervals.c:248
>  57 in traverse_intervals_noorder of intervals.c:248
>  58 in mark_interval_tree of alloc.c:1538
>  59 in process_mark_stack of alloc.c:7264
>  60 in mark_object of alloc.c:7504
>  61 in mark_glyph_matrix of alloc.c:6847
>  62 in mark_window of alloc.c:7079
>  63 in process_mark_stack of alloc.c:7307
>  64 in mark_objects of alloc.c:7512
>  65 in mark_vectorlike of alloc.c:6891
>  66 in mark_frame of alloc.c:7037
>  67 in process_mark_stack of alloc.c:7303
>  68 in mark_object of alloc.c:7504
>  69 in mark_interval_tree_1 of alloc.c:1529
>  70 in traverse_intervals_noorder of intervals.c:243
>  71 in mark_interval_tree of alloc.c:1538
>  72 in process_mark_stack of alloc.c:7264
>  73 in mark_object of alloc.c:7504
>  74 in mark_overlay of alloc.c:6933
>  75 in process_mark_stack of alloc.c:7355
>  76 in mark_objects of alloc.c:7512
>  77 in mark_vectorlike of alloc.c:6891
>  78 in mark_buffer of alloc.c:6954
>  79 in process_mark_stack of alloc.c:7299
>  80 in mark_object of alloc.c:7504
>  81 in mark_interval_tree_1 of alloc.c:1529
>  82 in traverse_intervals_noorder of intervals.c:243
>  83 in mark_interval_tree of alloc.c:1538
>  84 in process_mark_stack of alloc.c:7264
>  85 in mark_object of alloc.c:7504
>  86 in mark_char_table of alloc.c:6920
>  87 in mark_char_table of alloc.c:6917
>  88 in process_mark_stack of alloc.c:7341
>  89 in mark_object of alloc.c:7504
>  90 in mark_char_table of alloc.c:6920
>  91 in mark_char_table of alloc.c:6917
>  92 in process_mark_stack of alloc.c:7341
>  93 in mark_objects of alloc.c:7512
>  94 in mark_vectorlike of alloc.c:6891
>  95 in mark_buffer of alloc.c:6954
>  96 in process_mark_stack of alloc.c:7299
>  97 in mark_objects of alloc.c:7512
>  98 in mark_vectorlike of alloc.c:6891
>  99 in mark_buffer of alloc.c:6954
> 100 in process_mark_stack of alloc.c:7299
> 101 in mark_object of alloc.c:7504
> 102 in mark_char_table of alloc.c:6920
> 103 in mark_char_table of alloc.c:6917
> 104 in process_mark_stack of alloc.c:7341
> 105 in mark_objects of alloc.c:7512
> 106 in mark_vectorlike of alloc.c:6891
> 107 in mark_buffer of alloc.c:6954
> 108 in process_mark_stack of alloc.c:7299
> 109 in mark_objects of alloc.c:7512
> 110 in mark_vectorlike of alloc.c:6891
> 111 in mark_buffer of alloc.c:6954
> 112 in process_mark_stack of alloc.c:7299
> 113 in mark_objects of alloc.c:7512
> 114 in mark_vectorlike of alloc.c:6891
> 115 in mark_buffer of alloc.c:6954
> 116 in process_mark_stack of alloc.c:7299
> 117 in mark_objects of alloc.c:7512
> 118 in mark_vectorlike of alloc.c:6891
> 119 in mark_buffer of alloc.c:6954
> 120 in process_mark_stack of alloc.c:7299
> 121 in mark_objects of alloc.c:7512
> 122 in mark_vectorlike of alloc.c:6891
> 123 in mark_buffer of alloc.c:6954
> 124 in process_mark_stack of alloc.c:7299
> 125 in mark_object of alloc.c:7504
> 126 in mark_object_root_visitor of alloc.c:6396
> 127 in visit_vectorlike_root of alloc.c:6348
> 128 in visit_buffer_root of alloc.c:6362
> 129 in visit_static_gc_roots of alloc.c:6374
> 130 in garbage_collect of alloc.c:6598
> 131 in maybe_garbage_collect of alloc.c:6507
> 132 in maybe_gc of /home/spwhitton/src/emacs/primary/src/lisp.h:5929
> 133 in exec_byte_code of bytecode.c:787
> 134 in funcall_lambda of eval.c:3252
> 135 in funcall_general of eval.c:3044
> 136 in Ffuncall of eval.c:3093
> 137 in Fapply of eval.c:2718
> 138 in funcall_subr of eval.c:3184
> 139 in exec_byte_code of bytecode.c:812
> 140 in funcall_lambda of eval.c:3252
> 141 in funcall_general of eval.c:3044
> 142 in Ffuncall of eval.c:3093
> 143 in timer_check_2 of /home/spwhitton/src/emacs/primary/src/lisp.h:1178
> 144 in timer_check of keyboard.c:4866
> 145 in readable_events of keyboard.c:3591
> 146 in get_input_pending of keyboard.c:7869
> 147 in detect_input_pending_run_timers of keyboard.c:11573
> 148 in wait_reading_process_output of process.c:5838
> 149 in kbd_buffer_get_event of /home/spwhitton/src/emacs/primary/src/lisp.h:1178
> 150 in read_event_from_main_queue of keyboard.c:2330
> 151 in read_decoded_event_from_main_queue of keyboard.c:2394
> 152 in read_char of keyboard.c:3015
> 153 in read_key_sequence of keyboard.c:10743
> 154 in command_loop_1 of /home/spwhitton/src/emacs/primary/src/lisp.h:1178
> 155 in internal_condition_case of eval.c:1613
> 156 in command_loop_2 of keyboard.c:1168
> 157 in internal_catch of eval.c:1292
> 158 in command_loop of /home/spwhitton/src/emacs/primary/src/lisp.h:1178
> 159 in recursive_edit_1 of keyboard.c:754
> 160 in Frecursive_edit of keyboard.c:837
> 161 in main of emacs.c:2631

Thanks.  Would you mind running Emacs with this patch installed and
configured with the aforesaid option, and responding with backtraces
from any triggered assertion?

diff --git a/src/frame.c b/src/frame.c
index 7f4bf274ad9..a4b8ca207ee 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -4831,14 +4831,18 @@ gui_set_font (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
 	  /* Clean F's image cache of images whose values are derived
 	     from the font width.  */
 	  clear_image_cache (f, Qauto);
+	  verify_image_cache_refcount (f);
 	}
       else
 	{
 	  /* Release the current image cache, and reuse or allocate a
 	     new image cache with IWIDTH.  */
 	  FRAME_IMAGE_CACHE (f)->refcount--;
+	  FRAME_IMAGE_CACHE (f) = NULL;
+	  verify_image_cache_refcount (f);
 	  FRAME_IMAGE_CACHE (f) = share_image_cache (f);
 	  FRAME_IMAGE_CACHE (f)->refcount++;
+	  verify_image_cache_refcount (f);
 	}
     }
 
diff --git a/src/frame.h b/src/frame.h
index 1d920d1a6bc..eee694d6920 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1682,6 +1682,34 @@ IMAGE_OPT_FROM_ID (struct frame *f, int id)
   eassume (0 <= used);
   return 0 <= id && id < used ? FRAME_IMAGE_CACHE (f)->images[id] : NULL;
 }
+
+/* Abort if F's image cache's `refcount' field disagrees with the number
+   of frames holding references to the same.  */
+
+INLINE void
+verify_image_cache_refcount (f)
+     struct frame *f;
+{
+#ifdef ENABLE_CHECKING
+  int expected;
+  Lisp_Object tail, frame;
+
+  if (FRAME_IMAGE_CACHE (f))
+    {
+      expected = 0;
+
+      FOR_EACH_FRAME (tail, frame)
+	{
+	  if (FRAME_IMAGE_CACHE (XFRAME (frame))
+	      == FRAME_IMAGE_CACHE (f))
+	    expected++;
+	}
+
+      eassert (expected == FRAME_IMAGE_CACHE (f)->refcount);
+    }
+#endif /* ENABLE_CHECKING */
+}
+
 #endif
 \f
 /***********************************************************************
diff --git a/src/image.c b/src/image.c
index 2945447b962..9420c579d7b 100644
--- a/src/image.c
+++ b/src/image.c
@@ -3625,6 +3625,7 @@ cache_image (struct frame *f, struct image *img)
     {
       c = FRAME_IMAGE_CACHE (f) = share_image_cache (f);
       c->refcount++;
+      verify_image_cache_refcount (f);
     }
 
   /* Find a free slot in c->images.  */
diff --git a/src/xfaces.c b/src/xfaces.c
index 188dd4778bc..0e0172e1984 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -680,6 +680,7 @@ init_frame_faces (struct frame *f)
     {
       FRAME_IMAGE_CACHE (f) = share_image_cache (f);
       ++FRAME_IMAGE_CACHE (f)->refcount;
+      verify_image_cache_refcount (f);
     }
 #endif /* HAVE_WINDOW_SYSTEM */
 
@@ -709,6 +710,7 @@ free_frame_faces (struct frame *f)
       struct image_cache *image_cache = FRAME_IMAGE_CACHE (f);
       if (image_cache)
 	{
+	  verify_image_cache_refcount (f);
 	  --image_cache->refcount;
 	  if (image_cache->refcount == 0)
 	    free_image_cache (f);