bug#49248: 28.0.50; read-from-minibuffer returns a window-live-p error when its original window is deleted

bug#49248: 28.0.50; read-from-minibuffer returns a window-live-p error when its original window is deleted

[Thierry Volpiatto]

Hello, I have a bug report
https://github.com/emacs-helm/helm/issues/2424
where it seems a minibuffer not belong anymore to its window when this
one is deleted.
Here is what's happening:
I have a function that change the place of the window where originally
read-from-minibuffer was called, the window goes right, left, up and
down at each call of this function, but once we press RET to exit the
read-from-minibuffer an error is raised:

Wrong type argument: window-live-p, #<window 38>
(See backtrace in the link to helm issue mentionned above)

The error seems to come from read-from-minibuffer.

This always worked since emacs-24 to emacs-27.2.

Here is the function:

https://github.com/emacs-helm/helm/blob/master/helm.el#L6596

I wonder if this is a bug or an incompatible change in emacs-28, if so
what are the new directives in such case.

Thanks.


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2021-06-27 built on IPadS340
Repository revision: b8f9e58ef72402e69a1f0960816184d52e5d2d29
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Linux Mint 19.3

Configured using:
 'configure CFLAGS=-O8 --with-mailutils --with-cairo
 --with-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Elisp

Minor modes in effect:
  bug-reference-prog-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-ligature-mode: t
  ligature-mode: t
  psession-mode: t
  psession-savehist-mode: t
  global-git-gutter-mode: t
  git-gutter-mode: t
  display-time-mode: t
  winner-mode: t
  show-paren-mode: t
  helm-epa-mode: t
  helm-descbinds-mode: t
  override-global-mode: t
  helm-adaptive-mode: t
  helm-mode: t
  shell-dirtrack-mode: t
  helm-popup-tip-mode: t
  async-bytecomp-package-mode: t
  dired-async-mode: t
  minibuffer-depth-indicate-mode: t
  straight-use-package-mode: t
  straight-package-neutering-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/thierry/.emacs.d/straight/build/transient/transient hides /usr/local/share/emacs/28.0.50/lisp/transient

Features:
(epa-mail face-remap mu4e-thread-folding addressbook-bookmark
mu4e-config org-mu4e mu4e-contrib mu4e-patch mu4e mu4e-org mu4e-main
mu4e-view mu4e-view-gnus gnus-art mm-uu mml2015 mm-view mml-smime smime
dig gnus-sum gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap
nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range
gnus-win gnus nnheader mu4e-view-common mu4e-headers mu4e-compose
mu4e-context mu4e-draft mu4e-actions ido rfc2368 smtpmail mu4e-mark
mu4e-proc mu4e-utils doc-view jka-compr image-mode exif mu4e-lists
mu4e-message shr kinsoku svg dom flow-fill hl-line mu4e-vars mu4e-meta
shadow sort mail-extr emacsbug message rmc puny rfc822 mml mml-sec
gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 mm-util
mail-prsvr mail-utils helm-command image-file image-converter cl-print
help-fns smerge-mode view helm-dabbrev tramp-archive tramp-gvfs
tramp-cache zeroconf dbus helm-x-files helm-for-files helm-bookmark
bookmark conf-mode ledger-config ledger-mode ledger-check ledger-texi
ledger-test ledger-sort ledger-report ledger-reconcile ledger-occur
ledger-fonts ledger-fontify ledger-state ledger-complete ledger-schedule
ledger-init ledger-xact ledger-post ledger-exec ledger-navigate eshell
esh-cmd esh-ext esh-opt esh-proc esh-io esh-arg esh-module esh-groups
esh-util ledger-context ledger-commodities ledger-regex bug-reference
naquadah-theme solar cal-dst holidays hol-loaddefs tv-utils undo-tree
diff undo-tree-autoloads ligature ligature-autoloads
rainbow-mode-autoloads psession frameset wgrep-helm wgrep grep compile
text-property-search wgrep-autoloads log-view pcvs-util pcmpl-git
pcmpl-git-autoloads toc-org-autoloads cl-indent ffap
markdown-toc-autoloads markdown-mode-autoloads autocrypt ietf-drums
config-w3m w3m-autoloads git-gutter git-gutter-autoloads mule-util appt
diary-lib diary-loaddefs gud anaconda-mode xref project pythonic f dash
s anaconda-mode-autoloads pythonic-autoloads f-autoloads s-autoloads
emamux-autoloads magit-autoloads magit-section-autoloads
git-commit-autoloads with-editor-autoloads transient-autoloads
dash-autoloads pcomplete-extension pcmpl-unix pcmpl-gnu iterator
iedit-autoloads ledger-mode-autoloads wdired dired-extension org-config
ob-gnuplot org-crypt net-utils time all-the-icons all-the-icons-faces
data-material data-weathericons data-octicons data-fileicons
data-faicons data-alltheicons all-the-icons-autoloads winner w3m-wget
wget wget-sysdep autotest-mode autoconf-mode paren woman man ediff
ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init
ediff-util init-helm helm-fd epa derived epg epg-config helm-epa
helm-misc helm-imenu imenu helm-elisp-package package url-handlers
helm-find helm-org org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-footnote org-src ob-comint org-pcomplete org-list
org-faces org-entities noutline outline org-version ob-emacs-lisp
ob-core ob-eval org-table ol org-keys org-compat advice org-macs
org-loaddefs cal-menu calendar cal-loaddefs helm-external helm-net
browse-url xml url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util url-parse url-vars mailcap
isearch-light helm-descbinds cus-edit pp wid-edit helm-ls-git vc-git
diff-mode vc vc-dispatcher helm-ipython helm-elisp helm-eval edebug
backtrace find-func helm-info python tramp-sh use-package-bind-key
bind-key popup helm-adaptive diminish helm-mode helm-files filenotify
tramp tramp-loaddefs trampver tramp-integration files-x tramp-compat
shell pcomplete comint ansi-color ring parse-time iso8601 time-date
ls-lisp auth-source password-cache json map helm-buffers helm-occur
helm-tags helm-locate helm-grep helm-regexp format-spec helm-utils
helm-help helm-types use-package-diminish helm-extensions-autoloads
helm-config helm-autoloads helm async-bytecomp helm-global-bindings
helm-easymenu helm-source eieio eieio-core eieio-loaddefs
helm-multi-match helm-lib dired-async dired-aux dired dired-loaddefs
async emms-autoloads finder-inf use-package-core popup-autoloads
diminish-autoloads mb-depth server edmacro kmacro avoid cus-load
use-package-autoloads bind-key-autoloads straight-autoloads straight
comp comp-cstr warnings rx cl-extra info autoload radix-tree seq
easy-mmode cl-seq pcase subr-x byte-opt checkdoc lisp-mnt thingatpt
help-mode cl-macs gv bytecomp byte-compile cconv cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 576183 177686)
 (symbols 48 39767 34)
 (strings 32 151520 65945)
 (string-bytes 1 5536163)
 (vectors 16 80431)
 (vector-slots 8 2008218 250049)
 (floats 8 1375 1061)
 (intervals 56 3800 2423)
 (buffers 992 76))
<#secure method=pgpmime mode=sign>

-- 
Thierry

bug#49248: 28.0.50; read-from-minibuffer returns a window-live-p error when its original window is deleted

[martin rudalics]

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

 > Wrong type argument: window-live-p, #<window 38>
 > (See backtrace in the link to helm issue mentionned above)

Can you try the attached.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: minibuf.c.diff --]
[-- Type: text/x-patch; name="minibuf.c.diff", Size: 434 bytes --]

diff --git a/src/minibuf.c b/src/minibuf.c
index 00069eabbe..1b842b7721 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1210,7 +1210,7 @@ read_minibuf_unwind (void)
 		     WINDOW_FRAME (XWINDOW (minibuf_window))))
 	    Fset_frame_selected_window (selected_frame, prev, Qnil);
 	}
-      else
+      else if (WINDOW_LIVE_P (calling_window))
 	Fset_frame_selected_window (calling_frame, calling_window, Qnil);
     }

bug#49248: 28.0.50; read-from-minibuffer returns a window-live-p error when its original window is deleted

[Thierry Volpiatto]

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


martin rudalics <rudalics@gmx.at> writes:

>> Wrong type argument: window-live-p, #<window 38>
>> (See backtrace in the link to helm issue mentionned above)
>
> Can you try the attached.

Thanks Martin for quick answer and fix, yes it is now working.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

bug#49248: 28.0.50; read-from-minibuffer returns a window-live-p error when its original window is deleted

[martin rudalics]

 > Thanks Martin for quick answer and fix, yes it is now working.

Pushed to master now.  Apparently, exiting from the minibuffer may
select a frame no less than four times here:

#1 in read_minibuf_unwind
   if (!EQ (exp_MB_frame, saved_selected_frame)
       && !NILP (exp_MB_frame))
     do_switch_frame (exp_MB_frame, 0, 0, Qt); /* This also sets
					     minibuf_window */

#2 in read_minibuf_unwind
       else if (WINDOW_LIVE_P (calling_window))
	Fset_frame_selected_window (calling_frame, calling_window, Qnil);

#3 in read_minibuf_unwind
   /* Restore the selected frame. */
   if (!EQ (exp_MB_frame, saved_selected_frame)
       && !NILP (exp_MB_frame))
     do_switch_frame (saved_selected_frame, 0, 0, Qt);

#4 in read_minibuf
   if (FRAMEP (calling_frame)
       && FRAME_LIVE_P (XFRAME (calling_frame))
       && (!EQ (selected_frame, calling_frame)
	  || (WINDOW_LIVE_P (XFRAME (calling_frame)->minibuffer_window)
	      && !EQ (XWINDOW (XFRAME (calling_frame)->minibuffer_window)
		      ->frame,
		      calling_frame))))
     call2 (Qselect_frame_set_input_focus, calling_frame, Qnil);

These will confuse the hell out of any window manager (and of me).  Can
some knowledgeable soul tell us what all this code is supposed to do and
whether it could be refactored in some sane way?

Many thanks in advance, martin

bug#49248: 28.0.50; read-from-minibuffer returns a window-live-p error when its original window is deleted

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 29 Jun 2021 10:03:11 +0200
> Cc: 49248@debbugs.gnu.org
> 
> Apparently, exiting from the minibuffer may select a frame no less
> than four times here:
> 
> #1 in read_minibuf_unwind
>    if (!EQ (exp_MB_frame, saved_selected_frame)
>        && !NILP (exp_MB_frame))
>      do_switch_frame (exp_MB_frame, 0, 0, Qt); /* This also sets
> 					     minibuf_window */
> 
> #2 in read_minibuf_unwind
>        else if (WINDOW_LIVE_P (calling_window))
> 	Fset_frame_selected_window (calling_frame, calling_window, Qnil);
> 
> #3 in read_minibuf_unwind
>    /* Restore the selected frame. */
>    if (!EQ (exp_MB_frame, saved_selected_frame)
>        && !NILP (exp_MB_frame))
>      do_switch_frame (saved_selected_frame, 0, 0, Qt);
> 
> #4 in read_minibuf
>    if (FRAMEP (calling_frame)
>        && FRAME_LIVE_P (XFRAME (calling_frame))
>        && (!EQ (selected_frame, calling_frame)
> 	  || (WINDOW_LIVE_P (XFRAME (calling_frame)->minibuffer_window)
> 	      && !EQ (XWINDOW (XFRAME (calling_frame)->minibuffer_window)
> 		      ->frame,
> 		      calling_frame))))
>      call2 (Qselect_frame_set_input_focus, calling_frame, Qnil);
> 
> These will confuse the hell out of any window manager (and of me).  Can
> some knowledgeable soul tell us what all this code is supposed to do and
> whether it could be refactored in some sane way?

#1 and #3 are identical, so I think we could avoid #3 by testing the
selected frame against saved_selected_frame.

#2 is only done if "the previous minibuffer displayed in this miniwindow is
dead", so I don't see a problem there.

#4 is only done if read_minibuf_unwind didn't already do it, for
whatever reason, so it's just the code trying to be defensive.
(read_minibuf_unwind must re-select the original frame in case we QUIT
or signal an error during read_minibuf.)

HTH