bug#72487: calculator-electric-mode causes freeze on subsequent use of minibuffer

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

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

Attached find a patch that should guarantee that only minibuffers are
recorded in a minibuffer window's list of previous buffers.  A ChangeLog
appears below.  If there are no objections, I'll install this on master
in two weeks.

martin

For minibuffer windows record minibuffers only (Bug#72487)

* src/minibuf.c (zip_minibuffer_stacks): Use wset type
functions.  Call 'record-window-buffer' instead of
'push-window-buffer-onto-prev' to handle all sorts of buffers
shown in minibuffer windows in a uniform way.
(read_minibuf): Call 'record-window-buffer' instead of
'push-window-buffer-onto-prev' for same reason as previous.
* lisp/calculator.el (calculator-update-display)
(calculator-save-and-quit): Make sure calculator buffer is live
before operating on it.
* lisp/window.el (record-window-buffer): Handle case where
WINDOW is a minibuffer window: Unconditionally remove WINDOW's
buffer from WINDOW's list of previous buffers and push it if
and only if it is a live minibuffer (Bug#72487).  Do not run
'buffer-list-update-hook' if WINDOW is a minibuffer window.
(push-window-buffer-onto-prev): Make it an alias of
'record-window-buffer' so it will run the latter's checks.
(replace-buffer-in-windows): Handle minibuffer windows and
rewrite doc-string accordingly.
* doc/lispref/windows.texi (Buffers and Windows): Explain
handling of minibuffer windows in 'replace-buffer-in-windows'.

[-- Attachment #2: minibuffer-windows.diff --]
[-- Type: text/x-patch, Size: 20091 bytes --]

index 656a44dfcbf..541c91ddae2 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -2353,6 +2353,12 @@ Buffers and Windows
 that are only referenced by window configurations) and remove any
 @code{quit-restore} or @code{quit-restore-prev} parameters
 (@pxref{Window Parameters}) referencing that buffer.
+
+This function does not replace the buffer specified by
+@var{buffer-or-name} in any minibuffer window showing it, nor does it
+delete minibuffer windows or minibuffer frames.  It removes, however,
+that buffer from the lists of previous and next buffers of all
+minibuffer windows.
 @end deffn
 
 By default, @code{replace-buffer-in-windows} deletes only windows
diff --git a/lisp/calculator.el b/lisp/calculator.el
index ef1e6d8dbc3..a9fe76259a8 100644
--- a/lisp/calculator.el
+++ b/lisp/calculator.el
@@ -1059,49 +1059,50 @@ calculator-number-to-string
 (defun calculator-update-display (&optional force)
   "Update the display.
 If optional argument FORCE is non-nil, don't use the cached string."
-  (set-buffer calculator-buffer)
-  ;; update calculator-stack-display
-  (when (or force (not (eq (car calculator-stack-display)
-                           calculator-stack)))
-    (setq calculator-stack-display
-          (cons calculator-stack
-                (if calculator-stack
-                  (concat
-                   (let ((calculator-displayer
-                          (if (and calculator-displayers
-                                   (= 1 (length calculator-stack)))
-                            ;; customizable display for a single value
-                            (caar calculator-displayers)
-                            calculator-displayer)))
-                     (mapconcat 'calculator-number-to-string
-                                (reverse calculator-stack)
-                                " "))
-                   " "
-                   (and calculator-display-fragile
-                        calculator-saved-list
-                        ;; Hack: use `eq' to compare the number: it's a
-                        ;; flonum, so `eq' means that its the actual
-                        ;; number rather than a computation that had an
-                        ;; equal result (eg, enter 1,3,2, use "v" to see
-                        ;; the average -- it now shows "2" instead of
-                        ;; "2 [3]").
-                        (eq (car calculator-stack)
-                            (nth calculator-saved-ptr
-                                 calculator-saved-list))
-                        (if (= 0 calculator-saved-ptr)
-                          (format "[%s]" (length calculator-saved-list))
-                          (format "[%s/%s]"
-                                  (- (length calculator-saved-list)
-                                     calculator-saved-ptr)
-                                  (length calculator-saved-list)))))
-                  ""))))
-  (let ((inhibit-read-only t))
-    (erase-buffer)
-    (insert (calculator-get-display)))
-  (set-buffer-modified-p nil)
-  (goto-char (if calculator-display-fragile
-               (1+ (length calculator-prompt))
-               (1- (point)))))
+  (when (buffer-live-p calculator-buffer)
+    (set-buffer calculator-buffer)
+    ;; update calculator-stack-display
+    (when (or force (not (eq (car calculator-stack-display)
+                             calculator-stack)))
+      (setq calculator-stack-display
+            (cons calculator-stack
+                  (if calculator-stack
+                      (concat
+                       (let ((calculator-displayer
+                              (if (and calculator-displayers
+                                       (= 1 (length calculator-stack)))
+				  ;; customizable display for a single value
+				  (caar calculator-displayers)
+				calculator-displayer)))
+			 (mapconcat 'calculator-number-to-string
+                                    (reverse calculator-stack)
+                                    " "))
+                       " "
+                       (and calculator-display-fragile
+                            calculator-saved-list
+                            ;; Hack: use `eq' to compare the number: it's a
+                            ;; flonum, so `eq' means that its the actual
+                            ;; number rather than a computation that had an
+                            ;; equal result (eg, enter 1,3,2, use "v" to see
+                            ;; the average -- it now shows "2" instead of
+                            ;; "2 [3]").
+                            (eq (car calculator-stack)
+				(nth calculator-saved-ptr
+                                     calculator-saved-list))
+                            (if (= 0 calculator-saved-ptr)
+				(format "[%s]" (length calculator-saved-list))
+                              (format "[%s/%s]"
+                                      (- (length calculator-saved-list)
+					 calculator-saved-ptr)
+                                      (length calculator-saved-list)))))
+                    ""))))
+    (let ((inhibit-read-only t))
+      (erase-buffer)
+      (insert (calculator-get-display)))
+    (set-buffer-modified-p nil)
+    (goto-char (if calculator-display-fragile
+		   (1+ (length calculator-prompt))
+		 (1- (point))))))
 
 ;;;---------------------------------------------------------------------
 ;;; Stack computations
@@ -1553,17 +1554,18 @@ calculator-help
 (defun calculator-quit ()
   "Quit calculator."
   (interactive)
-  (set-buffer calculator-buffer)
-  (let ((inhibit-read-only t)) (erase-buffer))
-  (unless calculator-electric-mode
-    (ignore-errors
-      (while (get-buffer-window calculator-buffer)
-        (delete-window (get-buffer-window calculator-buffer)))))
-  (kill-buffer calculator-buffer)
-  (message "Calculator done.")
-  (if calculator-electric-mode
-    (throw 'calculator-done nil) ; will kill the buffer
-    (setq calculator-buffer nil)))
+  (when (buffer-live-p calculator-buffer)
+    (set-buffer calculator-buffer)
+    (let ((inhibit-read-only t)) (erase-buffer))
+    (unless calculator-electric-mode
+      (ignore-errors
+	(while (get-buffer-window calculator-buffer)
+          (delete-window (get-buffer-window calculator-buffer)))))
+    (kill-buffer calculator-buffer)
+    (message "Calculator done.")
+    (if calculator-electric-mode
+	(throw 'calculator-done nil)		; will kill the buffer
+      (setq calculator-buffer nil))))
 
 (defun calculator-save-and-quit ()
   "Quit the calculator, saving the result on the `kill-ring'."
diff --git a/lisp/window.el b/lisp/window.el
index f4226fa4428..368444fbee8 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4469,75 +4469,86 @@ delete-other-windows-vertically
 
 ;;; Windows and buffers.
 
-;; `prev-buffers' and `next-buffers' are two reserved window slots used
+;; 'prev-buffers' and 'next-buffers' are two reserved window slots used
 ;; for (1) determining which buffer to show in the window when its
 ;; buffer shall be buried or killed and (2) which buffer to show for
-;; `switch-to-prev-buffer' and `switch-to-next-buffer'.
+;; 'switch-to-prev-buffer' and 'switch-to-next-buffer'.
 
-;; `prev-buffers' consists of <buffer, window-start, window-point>
+;; 'prev-buffers' consists of <buffer, window-start, window-point>
 ;; triples.  The entries on this list are ordered by the time their
 ;; buffer has been removed from the window, the most recently removed
 ;; buffer's entry being first.  The window-start and window-point
-;; components are `window-start' and `window-point' at the time the
+;; components are 'window-start' and 'window-point' at the time the
 ;; buffer was removed from the window which implies that the entry must
-;; be added when `set-window-buffer' removes the buffer from the window.
+;; be added when 'set-window-buffer' removes the buffer from the window.
 
-;; `next-buffers' is the list of buffers that have been replaced
-;; recently by `switch-to-prev-buffer'.  These buffers are the least
-;; preferred candidates of `switch-to-prev-buffer' and the preferred
-;; candidates of `switch-to-next-buffer' to switch to.  This list is
+;; 'next-buffers' is the list of buffers that have been replaced
+;; recently by 'switch-to-prev-buffer'.  These buffers are the least
+;; preferred candidates of 'switch-to-prev-buffer' and the preferred
+;; candidates of 'switch-to-next-buffer' to switch to.  This list is
 ;; reset to nil by any action changing the window's buffer with the
-;; exception of `switch-to-prev-buffer' and `switch-to-next-buffer'.
-;; `switch-to-prev-buffer' pushes the buffer it just replaced on it,
-;; `switch-to-next-buffer' pops the last pushed buffer from it.
-
-;; Both `prev-buffers' and `next-buffers' may reference killed buffers
-;; if such a buffer was killed while the window was hidden within a
-;; window configuration.  Such killed buffers get removed whenever
-;; `switch-to-prev-buffer' or `switch-to-next-buffer' encounter them.
-
-;; The following function is called by `set-window-buffer' _before_ it
-;; replaces the buffer of the argument window with the new buffer.
-(defun push-window-buffer-onto-prev (&optional window)
-  "Push entry for WINDOW's buffer onto WINDOW's prev-buffers list.
-WINDOW must be a live window and defaults to the selected one.
-
-Any duplicate entries for the buffer in the list are removed."
+;; exception of 'switch-to-prev-buffer' and 'switch-to-next-buffer'.
+;; 'switch-to-prev-buffer' pushes the buffer it just replaced on it,
+;; 'switch-to-next-buffer' pops the last pushed buffer from it.
+
+;; The following function is called by 'set-window-buffer' _before_ it
+;; replaces the buffer of the argument window with the new buffer.  It
+;; does not record a non-minibuffer buffer (like the one created by
+;; 'calculator' in Electric mode) in a minibuffer window since the code
+;; in minibuf.c cannot handle that.  The minibuf.c code calls this
+;; function exclusively to arrange minibuffers shown in minibuffer
+;; windows.
+(defun record-window-buffer (&optional window)
+  "Record WINDOW's buffer.
+Add the buffer currently shown in WINDOW to the list of WINDOW's
+previous buffers.  WINDOW must be a live window and defaults to the
+selected one.
+
+If WINDOW is not a minibuffer window, do not record insignificant
+buffers (buffers whose name starts with a space).  If WINDOW is a
+minibuffer window, record its buffer if and only if that buffer is a
+live minibuffer (`minibufferp' with LIVE argument non-nil must return
+non-nil for it).
+
+Run `buffer-list-update-hook' if and only if WINDOW is not a minibuffer
+window."
   (let* ((window (window-normalize-window window t))
+	 (mini (window-minibuffer-p window))
          (buffer (window-buffer window))
-         (w-list (window-prev-buffers window))
-         (entry (assq buffer w-list)))
+         (prev-buffers (window-prev-buffers window))
+         (entry (assq buffer prev-buffers)))
     (when entry
-      (setq w-list (assq-delete-all buffer w-list)))
-    (let ((start (window-start window))
-          (point (window-point window)))
-      (setq entry
-            (cons buffer
-                  (with-current-buffer buffer
-                    (if entry
-                        ;; We have an entry, update marker positions.
-                        (list (set-marker (nth 1 entry) start)
-                              (set-marker (nth 2 entry) point))
-                      (list (copy-marker start)
-                            (copy-marker
-                             ;; Preserve window-point-insertion-type
-                             ;; (Bug#12855)
-                             point window-point-insertion-type))))))
-      (set-window-prev-buffers window (cons entry w-list)))))
+      (setq prev-buffers (assq-delete-all buffer prev-buffers)))
 
-(defun record-window-buffer (&optional window)
-  "Record WINDOW's buffer.
-WINDOW must be a live window and defaults to the selected one."
-  (let* ((window (window-normalize-window window t))
-         (buffer (window-buffer window)))
     ;; Reset WINDOW's next buffers.  If needed, they are resurrected by
     ;; `switch-to-prev-buffer' and `switch-to-next-buffer'.
     (set-window-next-buffers window nil)
 
-    ;; Don't record insignificant buffers.
-    (when (not (eq (aref (buffer-name buffer) 0) ?\s))
-      (push-window-buffer-onto-prev window)
-      (run-hooks 'buffer-list-update-hook))))
+    ;; For minibuffer windows record live minibuffers only.  For normal
+    ;; windows do not record insignificant buffers.
+    (when (if mini
+	      (minibufferp buffer t)
+	    (eq (aref (buffer-name buffer) 0) ?\s))
+      (let ((start (window-start window))
+            (point (window-point window)))
+	(setq entry
+              (cons buffer
+                    (with-current-buffer buffer
+                      (if entry
+                          ;; We have an entry, update marker positions.
+                          (list (set-marker (nth 1 entry) start)
+				(set-marker (nth 2 entry) point))
+			(list (copy-marker start)
+                              (copy-marker
+                               ;; Preserve window-point-insertion-type
+                               ;; (Bug#12855)
+                               point window-point-insertion-type))))))
+	(set-window-prev-buffers window (cons entry prev-buffers))
+
+	(unless mini
+	  (run-hooks 'buffer-list-update-hook))))))
+
+(defalias 'push-window-buffer-onto-prev 'record-window-buffer)
 
 (defun unrecord-window-buffer (&optional window buffer all)
   "Unrecord BUFFER in WINDOW.
@@ -5160,10 +5171,19 @@ delete-windows-on
 	;; If a window doesn't show BUFFER, unrecord BUFFER in it.
 	(unrecord-window-buffer window buffer t)))))
 
+;; Conceptually, 'replace-buffer-in-windows' would not have to touch the
+;; list of previous buffers of a minibuffer window: As a rule,
+;; minibuffers are never deleted and any other buffers shown in a
+;; minibuffer window are not recorded by 'record-window'.  To be on the
+;; safe side, 'replace-buffer-in-windows' now scans minibuffer windows
+;; too to make sure that any killed buffer gets removed from all lists
+;; of previous and next buffers.  'replace-buffer-in-windows' still does
+;; _not_ replace the buffer itself in any minibuffer window showing it.
+;; That case is still handled only in 'kill-buffer' itself.
 (defun replace-buffer-in-windows (&optional buffer-or-name)
   "Replace BUFFER-OR-NAME with some other buffer in all windows showing it.
 BUFFER-OR-NAME may be a buffer or the name of an existing buffer and
-defaults to the current buffer.  Minibuffer windows are not considered.
+defaults to the current buffer.
 
 If the option `kill-buffer-quit-windows' is nil, behave as follows: With
 the exception of side windows, when a window showing BUFFER-OR-NAME is
@@ -5180,21 +5200,29 @@ replace-buffer-in-windows
 lists of previous and next buffers of all windows and remove any
 `quit-restore' or `quit-restore-prev' parameters mentioning it.
 
+This function does not replace the buffer specified by BUFFER-OR-NAME in
+any minibuffer window showing it, nor does it delete minibuffer windows
+or minibuffer frames.  It removes, however, that buffer from the lists
+of previous and next buffers of all minibuffer windows.
+
 If, for any window showing BUFFER-OR-NAME running the abnormal hook
 `window-deletable-functions' returns nil, do not delete that window but
 show some other buffer in that window.
 
-This function is called by `kill-buffer' which kills the buffer
-specified by `buffer-or-name' afterwards.  It never kills a buffer by
-itself."
+This function is called by `kill-buffer' which effectively kills the
+buffer specified by `buffer-or-name' afterwards.  It never kills a
+buffer by itself."
   (interactive "bBuffer to replace: ")
   (let ((buffer (window-normalize-buffer buffer-or-name)))
-    ;; Scan all windows.  We have to unrecord BUFFER-OR-NAME in those
-    ;; not showing it.
-    (dolist (window (window-list-1 nil nil t))
+    ;; Scan all windows including minibuffer windows.  We have to
+    ;; unrecord BUFFER-OR-NAME even in those not showing it.
+    (dolist (window (window-list-1 nil t t))
       (when (eq (window-buffer window) buffer)
-	(if kill-buffer-quit-windows
-	    (quit-restore-window window 'killing)
+	(cond
+	 ((window-minibuffer-p window))
+	 (kill-buffer-quit-windows
+	  (quit-restore-window window 'killing))
+	 (t
 	  (let ((dedicated-side (eq (window-dedicated-p window) 'side)))
             (when (or dedicated-side (not (window--delete window t 'kill)))
 	      ;; Switch to another buffer in that window.
@@ -5212,7 +5240,7 @@ replace-buffer-in-windows
 	  ;; element of the parameter, 'quit-restore-window' cannot
 	  ;; possibly show BUFFER instead; so this parameter becomes
 	  ;; useless too.
-	  (unrecord-window-buffer window buffer t)))))
+	  (unrecord-window-buffer window buffer t))))))
 
 (defcustom quit-window-hook nil
   "Hook run before performing any other actions in the `quit-window' command."
diff --git a/src/minibuf.c b/src/minibuf.c
index f16880011f7..1f94e0e650e 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -160,16 +160,15 @@ zip_minibuffer_stacks (Lisp_Object dest_window, Lisp_Object source_window)
       set_window_buffer (dest_window, sw->contents, 0, 0);
       Fset_window_start (dest_window, Fwindow_start (source_window), Qnil);
       Fset_window_point (dest_window, Fwindow_point (source_window));
-      dw->prev_buffers = sw->prev_buffers;
+      wset_prev_buffers (dw, sw->prev_buffers);
       set_window_buffer (source_window, nth_minibuffer (0), 0, 0);
-      sw->prev_buffers = Qnil;
+      wset_prev_buffers (sw, Qnil);
       return;
     }
 
-  if (live_minibuffer_p (dw->contents))
-    call1 (Qpush_window_buffer_onto_prev, dest_window);
-  if (live_minibuffer_p (sw->contents))
-    call1 (Qpush_window_buffer_onto_prev, source_window);
+  call1 (Qrecord_window_buffer, dest_window);
+  call1 (Qrecord_window_buffer, source_window);
+
   acc = merge_c (dw->prev_buffers, sw->prev_buffers, minibuffer_ent_greater);
 
   if (!NILP (acc))
@@ -180,8 +179,9 @@ zip_minibuffer_stacks (Lisp_Object dest_window, Lisp_Object source_window)
       Fset_window_start (dest_window, Fcar (Fcdr (d_ent)), Qnil);
       Fset_window_point (dest_window, Fcar (Fcdr (Fcdr (d_ent))));
     }
-  dw->prev_buffers = acc;
-  sw->prev_buffers = Qnil;
+
+  wset_prev_buffers (dw, acc);
+  wset_prev_buffers (sw, Qnil);
   set_window_buffer (source_window, nth_minibuffer (0), 0, 0);
 }
 
@@ -688,8 +688,8 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 				    Fframe_first_window (MB_frame), Qnil);
     }
   MB_frame = XWINDOW (XFRAME (selected_frame)->minibuffer_window)->frame;
-  if (live_minibuffer_p (XWINDOW (minibuf_window)->contents))
-    call1 (Qpush_window_buffer_onto_prev, minibuf_window);
+
+  call1 (Qrecord_window_buffer, minibuf_window);
 
   record_unwind_protect_void (minibuffer_unwind);
   if (read_minibuffer_restore_windows)
diff --git a/src/window.c b/src/window.c
index 35092ddd582..34968ac824f 100644
--- a/src/window.c
+++ b/src/window.c
@@ -3647,9 +3647,17 @@ replace_buffer_in_windows (Lisp_Object buffer)
     call1 (Qreplace_buffer_in_windows, buffer);
 }
 
-/* If BUFFER is shown in a window, safely replace it with some other
-   buffer in all windows of all frames, even those on other keyboards.  */
-
+/** If BUFFER is shown in any window, safely replace it with some other
+    buffer in all windows of all frames, even those on other keyboards.
+    Do not delete any window.
+
+    This function is called by Fkill_buffer when it detects that
+    replacing BUFFER in some window showing BUFFER has failed.  It
+    assumes that ‘replace-buffer-in-windows’ has removed any entry
+    referencing BUFFER from any window's lists of previous and next
+    buffers and that window's ‘quit-restore’ and 'quit-restore-prev'
+    parameters.
+*/
 void
 replace_buffer_in_windows_safely (Lisp_Object buffer)
 {