bug#26682: 26.0.50; Error at startup with minibuffer-only frame

[martin rudalics <rudalics@gmx.at>]

[-- Attachment #1: Type: text/plain, Size: 6005 bytes --]

 > This seems to come from the following part at the beginning of
 > delete_frame:
 >
 >    else if (!EQ (force, Qnoelisp) && !other_frames (f, !NILP (force)))
 >      {
 >        if (NILP (force))
 > 	error ("Attempt to delete the sole visible or iconified frame");
 >        else
 > 	error ("Attempt to delete the only frame");
 >      }
 >
 > The original code there was:
 >
 >    if (NILP (force) && !other_visible_frames (f))
 >      error ("Attempt to delete the sole visible or iconified frame");
 >
 > Martin, why did you decide to disallow frame deletion when the FORCE
 > argument is t?  This makes delete-frame violate its documented
 > contract, whereby if FORCE == Qt, the fact that other frames are not
 > visible doesn't count.

I probably was confused.  One problem I wanted to fix is the following
scenario with Emacs 25.2: With emacs -Q evaluate the following four
forms in row:

(defvar old-frame (selected-frame))

(defvar new-frame (make-frame))

(add-hook 'delete-frame-functions
	  (lambda (f) (delete-frame new-frame)))

(delete-frame old-frame)

This gets me here:

Program received signal SIGTRAP, Trace/breakpoint trap.
0x7c911231 in ntdll!DbgUiConnectToDbg () from C:\WINDOWS\system32\ntdll.dll
(gdb) bt
#0  0x7c911231 in ntdll!DbgUiConnectToDbg () from C:\WINDOWS\system32\ntdll.dll
#1  0x012d28dd in emacs_abort () at w32fns.c:9830
#2  0x01323e14 in w32_reset_fringes () at fringe.c:1777
#3  0x012f1489 in x_delete_display (dpyinfo=0x2b1bc60) at w32term.c:6941
#4  0x012f1247 in x_delete_terminal (terminal=0x1a560b0) at w32term.c:6847
#5  0x01130bbe in Fdelete_terminal (terminal=..., force=...) at terminal.c:390
#6  0x01019d9c in delete_frame (frame=..., force=...) at frame.c:1731
#7  0x0101a55f in Fdelete_frame (frame=..., force=...) at frame.c:1842
#8  0x0121f0c2 in eval_sub (form=...) at eval.c:2174
#9  0x0121e235 in Feval (form=..., lexical=...) at eval.c:1993
#10 0x0122109f in Ffuncall (nargs=3, args=0x82e0e8) at eval.c:2701
#11 0x0127dbf5 in exec_byte_code (bytestr=..., vector=..., maxdepth=..., args_template=..., nargs=1, args=0x82e690) at bytecode.c:880
#12 0x01221e60 in funcall_lambda (fun=..., nargs=1, arg_vector=0x82e688) at eval.c:2860
#13 0x0122146e in Ffuncall (nargs=2, args=0x82e680) at eval.c:2747
#14 0x0127dbf5 in exec_byte_code (bytestr=..., vector=..., maxdepth=..., args_template=..., nargs=1, args=0x82ed78) at bytecode.c:880
#15 0x01221e60 in funcall_lambda (fun=..., nargs=1, arg_vector=0x82ed70) at eval.c:2860
#16 0x0122146e in Ffuncall (nargs=2, args=0x82ed68) at eval.c:2747
#17 0x01213432 in Ffuncall_interactively (nargs=2, args=0x82ed68) at callint.c:252
#18 0x01220f0f in Ffuncall (nargs=3, args=0x82ed60) at eval.c:2678
#19 0x0121665e in Fcall_interactively (function=..., record_flag=..., keys=...) at callint.c:843
#20 0x012210f6 in Ffuncall (nargs=4, args=0x82f1c8) at eval.c:2705
#21 0x0127dbf5 in exec_byte_code (bytestr=..., vector=..., maxdepth=..., args_template=..., nargs=1, args=0x82f780) at bytecode.c:880
#22 0x01221e60 in funcall_lambda (fun=..., nargs=1, arg_vector=0x82f778) at eval.c:2860
#23 0x0122146e in Ffuncall (nargs=2, args=0x82f770) at eval.c:2747
#24 0x012207bf in call1 (fn=..., arg1=...) at eval.c:2557
#25 0x01151cff in command_loop_1 () at keyboard.c:1479
#26 0x0121bd32 in internal_condition_case (bfun=0x1151233 <command_loop_1>, handlers=..., hfun=0x11504cb <cmd_error>) at eval.c:1314
#27 0x01150ccd in command_loop_2 (ignore=...) at keyboard.c:1107
#28 0x0121aed6 in internal_catch (tag=..., func=0x1150c91 <command_loop_2>, arg=...) at eval.c:1079
#29 0x01150c53 in command_loop () at keyboard.c:1086
#30 0x0114fee3 in recursive_edit_1 () at keyboard.c:692
#31 0x011501a4 in Frecursive_edit () at keyboard.c:763
#32 0x0114d8ba in main (argc=2, argv=0xa32658) at emacs.c:1626

Lisp Backtrace:
"delete-frame" (0x82ded0)
"eval" (0x82e0f0)
"elisp--eval-last-sexp" (0x82e688)
"eval-last-sexp" (0x82ed70)
"funcall-interactively" (0x82ed68)
"call-interactively" (0x82f1d0)
"command-execute" (0x82f778)

Admittedly calling ‘delete-frame’ from ‘delete-frame-functions’ is not
nice.  But with child frames and the ‘delete-before’ parameter it would
be easy to get a similar crash with a less provocative setup.

So I started to check for the existence of other frames twice and be
more rigorous.  Too rigorous, as Stefan's scenario demonstrates.

I attach a presumptive patch which should handle Stefan's setup and also
a few ‘make-frame-invisible’ calls which were not handled correctly.
Please try it.  It needs some further testing before I can commit it.

As a matter of fact, I was not able to repeat Stefan's scenario on
GNU/Linux - the initial frame was always deleted orderly.  But the
behavior is easily reproducible on Windows.  On Windows, however, even
with my patch, one out of ten times starting Emacs with Stefan's command
line hangs it with the initial frame visible and I have to quit with
C-g.

More precisely, it hangs in ‘frame-notice-user-settings’ in the while
loop below

	    ;; MS-Windows needs this to avoid inflooping below.
	    (if (eq system-type 'windows-nt)
		(sit-for 0 t))
	    ;; If the frame isn't visible yet, wait till it is.
	    ;; If the user has to position the window,
	    ;; Emacs doesn't know its real position until
	    ;; the frame is seen to be visible.
	    (while (not (cdr (assq 'visibility
				   (frame-parameters frame-initial-frame))))
	      (sleep-for 1))

and it does so always when I run Emacs under GDB.  I don't yet know why
it hangs: The request to make the frame visible is posted orderly but
somehow SET_FRAME_VISIBLE gets never executed.  Can you reproduce it?


Note also that the patch restores the Emasc 25.2 behavior where deleting
the last minibuffer frame with a mouse click raises the "surrogate ..."
error.  My earlier changes did ‘save-buffers-kill-emacs’ in that case.

martin

[-- Attachment #2: frame.c.el.diff --]
[-- Type: text/plain, Size: 4834 bytes --]

diff --git a/lisp/frame.el b/lisp/frame.el
index cec2624..8dad63f 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -123,8 +123,6 @@ handle-delete-frame
             ;; not a child frame.
             (when (and (not (eq frame-1 frame))
                        (frame-visible-p frame-1)
-                       (window-live-p (minibuffer-window frame-1))
-                       (eq (window-frame (minibuffer-window frame-1)) frame-1)
                        (not (frame-parent frame-1))
                        (not (frame-parameter frame-1 'delete-before)))
               (throw 'other-frame t))))
diff --git a/src/frame.c b/src/frame.c
index 681a245..b108e45 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1560,15 +1560,16 @@ of them (the selected terminal frame) is actually displayed.
  * Return true if there exists at least one visible or iconified frame
  * but F.  Return false otherwise.
  *
- * Always return false when all remaining frames are either tooltip or
- * child frames or frames with a non-nil `delete-before' parameter.  If
- * INVISIBLE is false, also return false when the minibuffer window of
- * all remaining frames is on F.
-
+ * INVISIBLE true means we are called from make_frame_invisible where
+ * such a frame must be visible or iconified.  INVISIBLE nil means we
+ * are called from delete_frame.  In that case FORCE true means that the
+ * visibility status of such a frame can be ignored.
+ *
  * If F is the terminal frame and we are using X, return true if at
- * least one X frame exists.  */
+ * least one X frame exists.
+ */
 static bool
-other_frames (struct frame *f, bool invisible)
+other_frames (struct frame *f, bool invisible, bool force)
 {
   Lisp_Object frames, frame, frame1;
   struct frame *f1;
@@ -1591,23 +1592,20 @@ of them (the selected terminal frame) is actually displayed.
 	    x_sync (f1);
 #endif
 	  if (NILP (Fframe_parameter (frame1, Qtooltip))
-	      /* Tooltips and child frames don't count.  */
+	      /* Tooltips and child frames count neither for
+		 invisibility nor for deletions.  */
 	      && !FRAME_PARENT_FRAME (f1)
 	      /* Frames with a non-nil `delete-before' parameter don't
-		 count - either they depend on us or they depend on a
-		 frame that we will have to find right here.  */
-	      && NILP (get_frame_param (f1, Qdelete_before))
-	      /* Frames whose minibuffer window is on F don't count
-		 unless INVISIBLE is set - in that case F is either made
-		 invisible and may be autoraised from such a frame or
-		 the FORCE argument of delete_frame was non-nil.  */
-	      && (invisible || NILP (minibuffer_window)
-		  || !EQ (FRAME_MINIBUF_WINDOW (f1), minibuffer_window))
-	      /* At least one visible/iconified frame must remain.  */
+		 count for deletions.  */
+	      && (invisible || NILP (get_frame_param (f1, Qdelete_before)))
+	      /* For invisibility and normal deletions, at least one
+		 visible or iconified frame must remain (Bug#26682).  */
 	      && (FRAME_VISIBLE_P (f1) || FRAME_ICONIFIED_P (f1)
-		  /* Allow deleting the terminal frame when at least one
-		     X frame exists.  */
-		  || (FRAME_WINDOW_P (f1) && !FRAME_WINDOW_P (f))))
+		  || (!invisible
+		      && (force
+			  /* Allow deleting the terminal frame when at
+			     least one X frame exists.  */
+			  || (FRAME_WINDOW_P (f1) && !FRAME_WINDOW_P (f))))))
 	    return true;
 	}
     }
@@ -1680,7 +1678,7 @@ of them (the selected terminal frame) is actually displayed.

   if (!FRAME_LIVE_P (f))
     return Qnil;
-  else if (!EQ (force, Qnoelisp) && !other_frames (f, !NILP (force)))
+  else if (!EQ (force, Qnoelisp) && !other_frames (f, false, true))
     {
       if (NILP (force))
 	error ("Attempt to delete the sole visible or iconified frame");
@@ -1752,7 +1750,7 @@ of them (the selected terminal frame) is actually displayed.
      one.  */
   if (!FRAME_LIVE_P (f))
     return Qnil;
-  else if (!EQ (force, Qnoelisp) && !other_frames (f, !NILP (force)))
+  else if (!EQ (force, Qnoelisp) && !other_frames (f, false, true))
     {
       if (NILP (force))
 	error ("Attempt to delete the sole visible or iconified frame");
@@ -2275,7 +2273,7 @@ of them (the selected terminal frame) is actually displayed.
 {
   struct frame *f = decode_live_frame (frame);

-  if (NILP (force) && !other_frames (f, true))
+  if (NILP (force) && !other_frames (f, true, false))
     error ("Attempt to make invisible the sole visible or iconified frame");

   /* Don't allow minibuf_window to remain on an invisible frame.  */
@@ -2878,6 +2876,9 @@ Each element of ALIST has the form (PARM . VALUE), where PARM is a symbol.
   struct frame *f = decode_live_frame (frame);
   Lisp_Object prop, val;

+  if (EQ (alist, Qt))
+    return Qnil;
+
   /* I think this should be done with a hook.  */
 #ifdef HAVE_WINDOW_SYSTEM
   if (FRAME_WINDOW_P (f))