Subject: bug#21428: 24.5; Crash of emacs on OS X, installed via homebrew cask

From: Eli Zaretskii <[email-masked]>
Date: Thu, 22 Oct 2015 20:05:31 +0300

> From: Rainer M Krug <[email-masked]>
> Cc: [email-masked]
> Date: Thu, 22 Oct 2015 18:22:43 +0200
> 
> > That seems like an entirely different crash.
> 
> That is good to know - I was using emacs for quite some time and no crash.

Great!  I'd like you to run it for a few more days.  You could run it
regularly, not under GDB, because I think the bug for which the GDB
setup was created will not happen anymore.

> Would it be possible to backport the changes which caused this error to
> the 24 branch?

Try the patch below (it should apply with some differences in line
numbers).  I cannot make any promises (the display code changed quite
a bit since then), but on first sight it looks like this should work
with Emacs 24.

> > Can you see what caused
> > it (is emacsframe a NULL pointer or something?), and also show a
> > backtrace?
> 
> Is this the info you are looking for: I am using emacs almost
> exclusively in full-screen view.

Yes, but I'm afraid I cannot interpret it.  Martin, any insights?

Thanks.

diff --git a/src/frame.c b/src/frame.c
index 98a7a57..6d596a4 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3651,6 +3651,10 @@ x_set_font (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
   /* Attempt to hunt down bug#16028.  */
   SET_FRAME_GARBAGED (f);
 
+  /* This is important if we are called by some Lisp as part of
+     redisplaying the frame, see redisplay_internal.  */
+  f->fonts_changed = true;
+
   recompute_basic_faces (f);
 
   do_pending_window_change (0);
diff --git a/src/xdisp.c b/src/xdisp.c
index a793157..986e13f 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13810,6 +13810,7 @@ redisplay_internal (void)
 	      bool gcscrollbars
 		/* Only GC scrollbars when we redisplay the whole frame.  */
 		= f->redisplay || !REDISPLAY_SOME_P ();
+	      bool f_redisplay_flag = f->redisplay;
 	      /* Mark all the scroll bars to be removed; we'll redeem
 		 the ones we want when we redisplay their windows.  */
 	      if (gcscrollbars && FRAME_TERMINAL (f)->condemn_scroll_bars_hook)
@@ -13853,6 +13854,20 @@ redisplay_internal (void)
 			goto retry_frame;
 		    }
 
+		  /* If the frame's redisplay flag was not set before
+		     we went about redisplaying its windows, but it is
+		     set now, that means we employed some redisplay
+		     optimizations inside redisplay_windows, and
+		     bypassed producing some screen lines.  But if
+		     f->redisplay is now set, it might mean the old
+		     faces are no longer valid (e.g., if redisplaying
+		     some window called some Lisp which defined a new
+		     face or redefined an existing face), so trying to
+		     use them in update_frame will segfault.
+		     Therefore, we must redisplay this frame.  */
+		  if (!f_redisplay_flag && f->redisplay)
+		    goto retry_frame;
+
 		  /* Prevent various kinds of signals during display
 		     update.  stdio is not robust about handling
 		     signals, which can cause an apparent I/O error.  */
@@ -13906,8 +13921,10 @@ redisplay_internal (void)
       /* Compare desired and current matrices, perform output.  */
 
     update:
-      /* If fonts changed, display again.  */
-      if (sf->fonts_changed)
+      /* If fonts changed, display again.  Likewise if redisplay_window_1
+	 above caused some change (e.g., a change in faces) that requires
+	 considering the entire frame again.  */
+      if (sf->fonts_changed || sf->redisplay)
 	goto retry;
 
       /* Prevent freeing of realized faces, since desired matrices are