bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Tobias Bading]

1. emacs -Q

2. Open a file under version control, e.g. lisp/vc/ediff-util.el

3. Selecting some text with the mouse seems to increase the chance of the
   error occurring, maybe even select some text in another application

4. M-x ediff-revision RET RET RET RET

5. q RET

Sometimes this results in error

set-frame-parameter: Wrong type argument: frame-live-p, #<dead frame Ediff 0x560ee1b46200>

With

(trace-function-background 'set-frame-parameter)
(trace-function-background 'x-dnd-after-move-frame)
(trace-function-background 'handle-move-frame)

prior to quitting ediff, the error is logged as

1 -> (handle-move-frame (move-frame (#<dead frame Ediff 0x5643a7425888>)))
| 2 -> (x-dnd-after-move-frame #<dead frame Ediff 0x5643a7425888>)
| | 3 -> (set-frame-parameter #<dead frame Ediff 0x5643a7425888> dnd-root-window-position nil)
| | 3 <- set-frame-parameter: !non-local\ exit!
| 2 <- x-dnd-after-move-frame: !non-local\ exit!
1 <- handle-move-frame: !non-local\ exit!

---

In GNU Emacs 29.0.90 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.20, cairo version 1.16.0) of 2023-05-05 built on omega
Repository revision: 21ec6c1d5cc7b6e1858f21de62a593e25d205383
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: Ubuntu 20.04.6 LTS

Configured using:
 'configure --with-modules --with-imagemagick --without-tree-sitter'

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

Important settings:
  value of $LC_COLLATE: C.UTF-8
  value of $LC_MONETARY: de_DE.UTF-8
  value of $LC_NUMERIC: de_DE.UTF-8
  value of $LC_TIME: de_DE.UTF-8
  value of $LANG: C.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Messages

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date subr-x vc
ediff-vers ediff ediff-merg ediff-mult ediff-wind ediff-diff ediff-help
ediff-init ediff-util vc-git diff-mode easy-mmode cl-loaddefs cl-lib
vc-dispatcher vc-svn bug-reference byte-opt gv bytecomp byte-compile rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode 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 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 nadvice seq simple cl-generic indonesian philippine
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 emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 58114 7770)
 (symbols 48 7445 0)
 (strings 32 20878 2062)
 (string-bytes 1 638147)
 (vectors 16 14015)
 (vector-slots 8 199200 12038)
 (floats 8 45 80)
 (intervals 56 467 0)
 (buffers 976 13))

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Eli Zaretskii]

> Date: Fri, 5 May 2023 19:02:10 +0200
> From: Tobias Bading <tbading@web.de>
> 
> 1. emacs -Q
> 
> 2. Open a file under version control, e.g. lisp/vc/ediff-util.el
> 
> 3. Selecting some text with the mouse seems to increase the chance of the
>    error occurring, maybe even select some text in another application
> 
> 4. M-x ediff-revision RET RET RET RET
> 
> 5. q RET
> 
> Sometimes this results in error
> 
> set-frame-parameter: Wrong type argument: frame-live-p, #<dead frame Ediff 0x560ee1b46200>
> 
> With
> 
> (trace-function-background 'set-frame-parameter)
> (trace-function-background 'x-dnd-after-move-frame)
> (trace-function-background 'handle-move-frame)
> 
> prior to quitting ediff, the error is logged as
> 
> 1 -> (handle-move-frame (move-frame (#<dead frame Ediff 0x5643a7425888>)))
> | 2 -> (x-dnd-after-move-frame #<dead frame Ediff 0x5643a7425888>)
> | | 3 -> (set-frame-parameter #<dead frame Ediff 0x5643a7425888> dnd-root-window-position nil)
> | | 3 <- set-frame-parameter: !non-local\ exit!
> | 2 <- x-dnd-after-move-frame: !non-local\ exit!
> 1 <- handle-move-frame: !non-local\ exit!
> 

Can you set debug-on-error non-nil, and post the full Lisp backtrace
from the error?

Thanks.

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Tobias Bading]

> Can you set debug-on-error non-nil, and post the full Lisp backtrace
> from the error?

Just now I couldn’t reproduce the bug in my tweaked Emacs 29. After a

sudo cpupower frequency-set --max 400MHz

I could. In my clean build of emacs-29 branch 21ec6c1d5c I was able to
reproduce the bug without any CPU throttling. The result is the same:

Debugger entered--Lisp error: (wrong-type-argument frame-live-p #<dead frame Ediff 0x55e8aa9e3760>)
  modify-frame-parameters(#<dead frame Ediff 0x55e8aa9e3760> ((dnd-root-window-position)))
  set-frame-parameter(#<dead frame Ediff 0x55e8aa9e3760> dnd-root-window-position nil)
  x-dnd-after-move-frame(#<dead frame Ediff 0x55e8aa9e3760>)
  run-hook-with-args(x-dnd-after-move-frame #<dead frame Ediff 0x55e8aa9e3760>)
  handle-move-frame((move-frame (#<dead frame Ediff 0x55e8aa9e3760>)))
  funcall-interactively(handle-move-frame (move-frame (#<dead frame Ediff 0x55e8aa9e3760>)))
  call-interactively(handle-move-frame nil [(move-frame (#<dead frame Ediff 0x55e8aa9e3760>))])
  command-execute(handle-move-frame nil [(move-frame (#<dead frame Ediff 0x55e8aa9e3760>))] t)

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Eli Zaretskii]

> Date: Fri, 5 May 2023 19:02:10 +0200
> From: Tobias Bading <tbading@web.de>
> 
> 1. emacs -Q
> 
> 2. Open a file under version control, e.g. lisp/vc/ediff-util.el
> 
> 3. Selecting some text with the mouse seems to increase the chance of the
>    error occurring, maybe even select some text in another application
> 
> 4. M-x ediff-revision RET RET RET RET
> 
> 5. q RET
> 
> Sometimes this results in error
> 
> set-frame-parameter: Wrong type argument: frame-live-p, #<dead frame Ediff 0x560ee1b46200>
> 
> With
> 
> (trace-function-background 'set-frame-parameter)
> (trace-function-background 'x-dnd-after-move-frame)
> (trace-function-background 'handle-move-frame)
> 
> prior to quitting ediff, the error is logged as
> 
> 1 -> (handle-move-frame (move-frame (#<dead frame Ediff 0x5643a7425888>)))
> | 2 -> (x-dnd-after-move-frame #<dead frame Ediff 0x5643a7425888>)
> | | 3 -> (set-frame-parameter #<dead frame Ediff 0x5643a7425888> dnd-root-window-position nil)
> | | 3 <- set-frame-parameter: !non-local\ exit!
> | 2 <- x-dnd-after-move-frame: !non-local\ exit!
> 1 <- handle-move-frame: !non-local\ exit!

Does the patch below give good results?

diff --git a/lisp/x-dnd.el b/lisp/x-dnd.el
index 9286a18..acfbbed 100644
--- a/lisp/x-dnd.el
+++ b/lisp/x-dnd.el
@@ -609,8 +609,9 @@ x-dnd-init-xdnd-for-frame
 (defun x-dnd-after-move-frame (frame)
   "Handle FRAME moving to a different position.
 Clear any cached root window position."
-  (set-frame-parameter frame 'dnd-root-window-position
-                       nil))
+  (and (frame-live-p frame)
+       (set-frame-parameter frame 'dnd-root-window-position
+                            nil)))
 
 (add-hook 'move-frame-functions #'x-dnd-after-move-frame)
 

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Tobias Bading]

> Does the patch below give good results?
>
> diff --git a/lisp/x-dnd.el b/lisp/x-dnd.el
> index 9286a18..acfbbed 100644
> --- a/lisp/x-dnd.el
> +++ b/lisp/x-dnd.el
> @@ -609,8 +609,9 @@ x-dnd-init-xdnd-for-frame
>  (defun x-dnd-after-move-frame (frame)
>    "Handle FRAME moving to a different position.
>  Clear any cached root window position."
> -  (set-frame-parameter frame 'dnd-root-window-position
> -                       nil))
> +  (and (frame-live-p frame)
> +       (set-frame-parameter frame 'dnd-root-window-position
> +                            nil)))
>
>  (add-hook 'move-frame-functions #'x-dnd-after-move-frame)

It probably would, because for testing purposes I already patched
handle-move-frame like this:

diff --git a/lisp/frame.el b/lisp/frame.el
index bf984da0d6..d6ca63ce53 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -237,11 +237,16 @@ handle-focus-out
 (defun handle-move-frame (event)
   "Handle a move-frame event.
 This function runs the abnormal hook `move-frame-functions'."
   (interactive "e")
   (let ((frame (posn-window (event-start event))))
-    (run-hook-with-args 'move-frame-functions frame)))
+    ;; make sure the frame is still alive (TODO)
+    (if (frame-live-p frame)
+        (run-hook-with-args 'move-frame-functions frame)
+      (let ((inhibit-message t))
+        (message "FRAME IS ALREADY DEAD: %s" frame))
+      )))

Basically the same change, just one function higher and with a silent log
message. This successfully suppresses the error and logs when the strange
situation occurred.

This looks like some race between Emacs and Compiz or something. Sometimes
it occurs, sometimes it doesn’t. handle-move-frame should probably not get
triggered at all if the frame is already dead. So far I didn’t find the C
function doing this, but I didn’t look that hard because I have a few other
minor problems that might be related:
I need (ediff-control-frame-upward-shift -20) to properly align the ediff
control frame with the main frame. Also, starting Emacs with -geometry and
fiddling with left/top/width/height in default-frame-alist and
initial-frame-alist doesn’t give the same results as with Emacs 26.3.
Something fishy is going on and probably related to the fact that I’m using
a 4K monitor and thus HiDPI scaling is in play. Might be another case of a
missing conversion between GTK application pixels and device pixels
somewhere. Such a misinterpretation (or rounding error) of pixel positions
and/or sizes may trigger the handle-move-frame, which in itself doesn’t make
much sense when a frame is being destroyed.

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Eli Zaretskii]

> Date: Sat, 6 May 2023 12:10:33 +0200
> Cc: 63312@debbugs.gnu.org
> From: Tobias Bading <tbading@web.de>
> 
> > Does the patch below give good results?
> >
> > diff --git a/lisp/x-dnd.el b/lisp/x-dnd.el
> > index 9286a18..acfbbed 100644
> > --- a/lisp/x-dnd.el
> > +++ b/lisp/x-dnd.el
> > @@ -609,8 +609,9 @@ x-dnd-init-xdnd-for-frame
> >  (defun x-dnd-after-move-frame (frame)
> >    "Handle FRAME moving to a different position.
> >  Clear any cached root window position."
> > -  (set-frame-parameter frame 'dnd-root-window-position
> > -                       nil))
> > +  (and (frame-live-p frame)
> > +       (set-frame-parameter frame 'dnd-root-window-position
> > +                            nil)))
> >
> >  (add-hook 'move-frame-functions #'x-dnd-after-move-frame)
> 
> It probably would, because for testing purposes I already patched
> handle-move-frame like this:

Not really, because there could be other functions on the hook
variable, which do deal well with a dead frame.

Po Lu, any objections to installing my proposed change?

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 6 May 2023 12:10:33 +0200
>> Cc: 63312@debbugs.gnu.org
>> From: Tobias Bading <tbading@web.de>
>> 
>> > Does the patch below give good results?
>> >
>> > diff --git a/lisp/x-dnd.el b/lisp/x-dnd.el
>> > index 9286a18..acfbbed 100644
>> > --- a/lisp/x-dnd.el
>> > +++ b/lisp/x-dnd.el
>> > @@ -609,8 +609,9 @@ x-dnd-init-xdnd-for-frame
>> >  (defun x-dnd-after-move-frame (frame)
>> >    "Handle FRAME moving to a different position.
>> >  Clear any cached root window position."
>> > -  (set-frame-parameter frame 'dnd-root-window-position
>> > -                       nil))
>> > +  (and (frame-live-p frame)
>> > +       (set-frame-parameter frame 'dnd-root-window-position
>> > +                            nil)))
>> >
>> >  (add-hook 'move-frame-functions #'x-dnd-after-move-frame)
>> 
>> It probably would, because for testing purposes I already patched
>> handle-move-frame like this:
>
> Not really, because there could be other functions on the hook
> variable, which do deal well with a dead frame.
>
> Po Lu, any objections to installing my proposed change?

Please install on emacs-29, thanks.  This hook should not be called with
a dead frame though, and Martin proposed a fix for that which was
installed last year, so I will continue to investigate on master.

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Tobias Bading <tbading@web.de>,  63312@debbugs.gnu.org
> Date: Sat, 06 May 2023 19:12:51 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Not really, because there could be other functions on the hook
> > variable, which do deal well with a dead frame.
> >
> > Po Lu, any objections to installing my proposed change?
> 
> Please install on emacs-29, thanks.  This hook should not be called with
> a dead frame though, and Martin proposed a fix for that which was
> installed last year, so I will continue to investigate on master.

Done.  Should I close this bug now, or do you want it to stay open?

bug#63312: 29.0.90; set-frame-parameter sometimes called with dead frame when quitting ediff

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: Tobias Bading <tbading@web.de>,  63312@debbugs.gnu.org
>> Date: Sat, 06 May 2023 19:12:51 +0800
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Not really, because there could be other functions on the hook
>> > variable, which do deal well with a dead frame.
>> >
>> > Po Lu, any objections to installing my proposed change?
>> 
>> Please install on emacs-29, thanks.  This hook should not be called with
>> a dead frame though, and Martin proposed a fix for that which was
>> installed last year, so I will continue to investigate on master.
>
> Done.  Should I close this bug now, or do you want it to stay open?

I've just closed it, since what remains to be fixed is really another
problem.  Thanks.