Subject: bug#39977: 28.0.50; Unhelpful stack trace

From: martin rudalics <[email-masked]>
Date: Sun, 22 Mar 2020 19:20:33 +0100

>> Not really.  It's easy for delete_frame to refuse deleting a frame right
 >> at the beginning.  But once it has accepted a deletion, it might become
 >> hard to deal with all the consequences.
 >
 > I don't think I understand where you are going with this.

Once an initial frame has been created, Lisp code should be always able
to rely on the truth of

(and (frame-live-p (selected-frame))
      (window-live-p (selected-window))
      (eq (frame-selected-window (selected-frame))
	 (selected-window)))

Whenever this basic invariant is violated, there is no guarantee that
frame and window management will produce correct results.  Currently,
this invariance is no longer guaranteed if Lisp code is allowed to
manipulate frames and windows arbitrarily while processing the mode line
or the frame title.  IMO there are three possible ways to deal with this
problem:

(1) Let the redisplay code handle it.

(2) Let the frame and window management handle it by disallowing such
operations while they are issued by the mode line or frame title
processing code.

(3) Ignore it and let the frame/window management routines catch up with
it later.

Using (1) way my initial idea.  The patch I proposed handles simple
cases like Madhu's bug.  It will certainly not handle more sophisticated
cases where, for example, an application kills two frames in a row.

(2) is by far the most simple and reliable approach but it will restrict
applications in what they are allowed to do when processing a mode line
or frame title.

(3) means that frame/window management proceeds in a non-deterministic
fashion as long as it has not detected that its basic invariant has been
violated.

 > This isn't about trust.  This is about letting users' Lisp do anything
 > they want as long as the results allow redisplay to continue after
 > that Lisp returns.  I don't think it's right to disallow certain
 > actions just because they _might_ cause problems.

You again care about redisplay only.  Which means that frame/window
management is second-class as far as safety is concerned.

 >>   > No, they are there in cases where we simply don't know how to
 >>   > continue.
 >>
 >> If that's the reason, then SELECTED_FRAME can easily set selected_frame
 >> to some live frame and continue.
 >
 > Something like that, yes.

I attach a patch that does that.  If you try it with a recipe like
loading

(defvar foo
   '(:eval
     (when (> (length (frame-list)) 1)
       (delete-frame (next-frame)))))

(setq-default mode-line-format foo)

(make-frame)

with emacs -Q you will see that while it works around the crash it still
produces a

Wrong type argument: window-live-p, #<window 3>

error in redisplay.

martin
--- a/src/frame.c
+++ b/src/frame.c
@@ -1843,6 +1843,36 @@ other_frames (struct frame *f, bool invisible, bool force)
   return false;
 }
 
+
+/** Select some live frame.  */
+bool
+select_some_frame (void)
+{
+  Lisp_Object tail;
+  Lisp_Object frame UNINIT;
+
+  FOR_EACH_FRAME (tail, frame)
+    {
+      struct frame *f = XFRAME (frame);
+
+      if (!FRAME_PARENT_FRAME (f) && !FRAME_TOOLTIP_P (f))
+	{
+	  /* Select FRAME without switching to it.  This sets up the
+	     selected frame and the selected window and avoids aborting
+	     as when redisplay selects a dead frame (Bug#39977).  */
+	  selected_frame = frame;
+	  if (!WINDOW_LIVE_P (FRAME_SELECTED_WINDOW (f)))
+	    FRAME_SELECTED_WINDOW (f) = Fframe_first_window (frame);
+	  selected_window = FRAME_SELECTED_WINDOW (f);
+
+	  return true;
+	}
+    }
+
+  return false;
+}
+
+
 /* Make sure that minibuf_window doesn't refer to FRAME's minibuffer
    window.  Preferably use the selected frame's minibuffer window
    instead.  If the selected frame doesn't have one, get some other
diff --git a/src/frame.h b/src/frame.h
index a54b8623e5..807938fd9f 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1363,14 +1363,17 @@ window_system_available (struct frame *f)
 
 extern Lisp_Object Vframe_list;
 
+bool select_some_frame (void);
+
 /* Value is a pointer to the selected frame.  If the selected frame
    isn't live, abort.  */
 
 #define SELECTED_FRAME()				\
-     ((FRAMEP (selected_frame)				\
-       && FRAME_LIVE_P (XFRAME (selected_frame)))	\
-      ? XFRAME (selected_frame)				\
-      : (emacs_abort (), (struct frame *) 0))
+  (((FRAMEP (selected_frame)				\
+     && FRAME_LIVE_P (XFRAME (selected_frame)))		\
+    || select_some_frame ())				\
+   ? XFRAME (selected_frame)				\
+   : (emacs_abort (), (struct frame *) 0))
 
 
 /***********************************************************************