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

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

[Kevin J Witczak via Bug reports for GNU Emacs, the Swiss army knife of text editors]

When using the calculator function of GNU Emacs 31.0.50 (bug also found 
in 29.4), usage of `calculator-electric-mode' causes Emacs to hang on 
subsequent uses of the minibuffer.

Emacs -Q

M-: (setf calculator-electric-mode t) <RET>

M-x calcu <RET> q M-x M-p <RET>

Note that the freeze does not depend on specifically calling calculator 
a second time, and calculator may be repeated with C-x z without issue.

The freeze occurs specifically when the minibuffer exits a second time, 
even via C-g, before its contents are processed. Thus, for example, the 
following still yields a freeze:

Emacs -Q

M-: (setf calculator-electric-mode t) <RET>

M-x d-o-e <RET> kill-emacs <RET>

M-x calcu <RET> q M-x kill-emacs <RET>

Apparently, calculator-electric-mode leaves the minibuffer window in a 
broken state after first usage, rather than resetting it correctly.

Emacs must be killed externally at this point, and generally cannot be 
salvaged with C-g. It becomes trapped in a loop emitting terminal bells 
(seen with `visible-bell' mode on, or via a termscript from `-nw').

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]

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

 > When using the calculator function of GNU Emacs 31.0.50 (bug also found
 > in 29.4), usage of `calculator-electric-mode' causes Emacs to hang on
 > subsequent uses of the minibuffer.
 >
 > Emacs -Q
 >
 > M-: (setf calculator-electric-mode t) <RET>
 >
 > M-x calcu <RET> q M-x M-p <RET>
 >
 > Note that the freeze does not depend on specifically calling calculator
 > a second time, and calculator may be repeated with C-x z without issue.
 >
 > The freeze occurs specifically when the minibuffer exits a second time,
 > even via C-g, before its contents are processed. Thus, for example, the
 > following still yields a freeze:
 >
 > Emacs -Q
 >
 > M-: (setf calculator-electric-mode t) <RET>
 >
 > M-x d-o-e <RET> kill-emacs <RET>
 >
 > M-x calcu <RET> q M-x kill-emacs <RET>
 >
 > Apparently, calculator-electric-mode leaves the minibuffer window in a
 > broken state after first usage, rather than resetting it correctly.
 >
 > Emacs must be killed externally at this point, and generally cannot be
 > salvaged with C-g. It becomes trapped in a loop emitting terminal bells
 > (seen with `visible-bell' mode on, or via a termscript from `-nw').

This bug is a consequence of accessing the list of previous buffers for
a minibuffer window.  That list is conceptually unmaintained and
therefore unsafe for minibuffer windows.  In particular, 'kill-buffer'
and subsequently 'replace-buffer-in-windows' do not consider minibuffer
windows.  The doc-string for the latter now states explicitly that
"Minibuffer windows are not considered."  Handling minibuffer windows in
'replace-buffer-in-windows' might require some deep surgery and is
certainly not recommended for a release branch.

I can offer the attached patch to provisionally handle this problem.

martin

[-- Attachment #2: minibuf.diff --]
[-- Type: text/x-patch, Size: 1818 bytes --]

diff --git a/src/minibuf.c b/src/minibuf.c
index 1dfee0a59c9..8932c7cd749 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1248,27 +1248,39 @@ read_minibuf_unwind (void)
 minibuffer_unwind (void)
 {
   struct frame *f;
-  struct window *w;
-  Lisp_Object window;
-  Lisp_Object entry;
 
   if (NILP (exp_MB_frame)) return; /* "Can't happen." */
   f = XFRAME (exp_MB_frame);
-  window = f->minibuffer_window;
-  w = XWINDOW (window);
   if (FRAME_LIVE_P (f))
     {
-      /* minibuf_window = sf->minibuffer_window; */
-      if (!NILP (w->prev_buffers))
+      Lisp_Object window = f->minibuffer_window;
+
+      if (WINDOW_LIVE_P (window))
 	{
-	  entry = Fcar (w->prev_buffers);
-	  w->prev_buffers = Fcdr (w->prev_buffers);
-	  set_window_buffer (window, Fcar (entry), 0, 0);
-	  Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
-	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
+	  struct window *w = XWINDOW (window);
+
+	  /* minibuf_window = sf->minibuffer_window; */
+	  if (!NILP (w->prev_buffers))
+	    {
+	      Lisp_Object entry = Fcar (w->prev_buffers);
+
+	      if (BUFFERP (Fcar (entry))
+		  && BUFFER_LIVE_P (XBUFFER (Fcar (entry))))
+		{
+		  w->prev_buffers = Fcdr (w->prev_buffers);
+		  set_window_buffer (window, Fcar (entry), 0, 0);
+		  Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
+		  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
+		  /* set-window-configuration may/will have unselected the
+		     mini-window as the selected window.  Restore it. */
+		  Fset_frame_selected_window (exp_MB_frame, window, Qnil);
+		}
+	      else
+		set_window_buffer (window, nth_minibuffer (0), 0, 0);
+	    }
+	  else
+	    set_window_buffer (window, nth_minibuffer (0), 0, 0);
 	}
-      else
-	set_window_buffer (window, nth_minibuffer (0), 0, 0);
     }
 }
 

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]

 > I can offer the attached patch to provisionally handle this problem.

I now pushed it to the release branch.  Please check again with your
scenarios.

The patch does not fix a conceptual discrepancy between minibuffers and
their depth.  As an example, try with emacs -Q:

C-h f

M-x

(setq foo (window-buffer (minibuffer-window)))

C-h v

(kill-buffer foo)

Now C-g will tell me

completing-read-default: Selecting deleted buffer [2 times]

And ESC ESC ESC gets me

keyboard-escape-quit: No catch for tag: exit, t [6 times]
minibuffer-quit-recursive-edit: No catch for tag: exit, #[0 "\301\300\242S!\207" [(2) minibuffer-quit-recursive-edit] 2]

so I will be trapped in these for the rest of my session.

Since I do not consider killing minibuffers as something reasonable to
do, I won't try to fix this for the release branch.  The most simple
solution might be to inhibit killing of buffers used by the minibuffer
code.

martin

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

[Eli Zaretskii]

> Date: Fri, 23 Aug 2024 10:42:46 +0200
> From:  martin rudalics via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
>  > I can offer the attached patch to provisionally handle this problem.
> 
> I now pushed it to the release branch.

Unless this is a recent regression (in Emacs 29 or since Emacs 29),
the release branch is not the right place to install this.  In the
latter case, please revert and install on master instead.

Thanks.

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]

 > Unless this is a recent regression (in Emacs 29 or since Emacs 29),
 > the release branch is not the right place to install this.  In the
 > latter case, please revert and install on master instead.

It's a regression in Emacs 29.

I spent about two hours hunting down this bug which misuses a feature I
introduced earlier in a very awkward way.  I'm still not sure how to fix
this completely.  That said, I think that it would be fair if, whenever
someone offers a patch for a reported serious bug (a bug that causes
Emacs to hang or crash), one of our maintainers explained within a week
or so how to proceed with the patch.

Thanks, martin

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

[Eli Zaretskii]

> Date: Sat, 24 Aug 2024 10:00:24 +0200
> Cc: kim.jet.wav@protonmail.ch, 72487@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Unless this is a recent regression (in Emacs 29 or since Emacs 29),
>  > the release branch is not the right place to install this.  In the
>  > latter case, please revert and install on master instead.
> 
> It's a regression in Emacs 29.

In that case, let's leave the fix on the release branch.  Thanks.

> I spent about two hours hunting down this bug which misuses a feature I
> introduced earlier in a very awkward way.  I'm still not sure how to fix
> this completely.  That said, I think that it would be fair if, whenever
> someone offers a patch for a reported serious bug (a bug that causes
> Emacs to hang or crash), one of our maintainers explained within a week
> or so how to proceed with the patch.

I usually respond after 2 weeks if no one else chimes in, so your
installing of the patch beat me by a couple of days.

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]

[-- 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)
 {

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]

 > 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.

Installed on master.  If there are no further complaints, I'll close
this bug in a couple of days.

Belated and overdue thanks for the report.  I was clearly annoyed by the
fact that such stupid behavior could have crept into the code base.

martin

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]

> Installed on master.  If there are no further complaints, I'll close
> this bug in a couple of days.

Done.

martin