bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Mikhail P]

[-- Attachment #1.1: Type: text/plain, Size: 7700 bytes --]

Greetings.

I am experiencing problems with viewing remote images. When resizing an 
image (e.g. by resizing Emacs frame) Emacs occasionally (approximately 
~30% reproducibility) can prompt |... changed on disk; really edit the 
buffer?| which does not make a lot of sense. Regardless of my response 
Emacs outputs errors, lags for several seconds and only after that 
resizes the image.

I am using currently most recent commit on TRAMP repo 
(69844458e33b5dcae53de249d9d82c59a5876055) containing some necessary 
fixes. Prior to these fixes Emacs could occasionally stall on resizing 
remote image and only interruption with C-g could help (or sometimes 
making Emacs completely unresponsive). Also attaching TRAMP debug file 
which I hope exposes the problem.

As far as I could understand from my conversation with TRAMP devs while 
working on the fixes there are problems with functions called on 
idle-timer in image mode ... 😬

Below as a quotation is what |report-emacs-bug| has generated.

To make this report I ran emacs as follows:

|emacs -Q --eval="(progn (add-to-list 'load-path 
\"/home/mikpom/Dropbox/.config/emacs-plugins/tramp\") (require 'tramp) 
(setq tramp-verbose 10) (setq tramp-use-ssh-controlmaster-options nil) 
(setq tramp-debug-to-file t))" 
/ssh:horsehop:/storage1/mikpom/wizard_devel/CNV/figs/TCGA_CNV/ATP23/TCGA-02-2485.png|

Thanks,

Mikhail


> In GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.22, 
> cairo version 1.17.3)
>  of 2020-08-29 built on juergen
> Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
> System Description: Arch Linux
>
> Recent messages:
> Unchanged content check: (remote-file-error "Forbidden reentrant call 
> of Tramp")
> error: "Command attempted to use minibuffer while in minibuffer"
> Error running timer ‘image-fit-to-window’: (error "Command attempted 
> to use minibuffer while in minibuffer")
> TCGA-02-2485.png changed on disk; really edit the buffer? (y, n, r or 
> C-h) n
> peculiar error: File changed on disk 
> /ssh:horsehop:/storage1/mikpom/wizard_devel/CNV/figs/TCGA_CNV/ATP23/TCGA-02-2485.png
> Error running timer ‘image-fit-to-window’: (file-supersession "File 
> changed on disk 
> /ssh:horsehop:/storage1/mikpom/wizard_devel/CNV/figs/TCGA_CNV/ATP23/TCGA-02-2485.png")
> Quit: "Quit", ""
> Quit
> Remote file error (compat): Forbidden reentrant call of Tramp [5 times]
> Error running timer ‘image-fit-to-window’: (remote-file-error 
> "Forbidden reentrant call of Tramp")
>
> Configured using:
>  'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
>  --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-wide-int
>  --with-modules --with-cairo --with-harfbuzz 'CFLAGS=-march=x86-64
>  -mtune=generic -O2 -pipe -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
>  LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'
>
> Configured features:
> XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
> INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB
> TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON
> PDUMPER LCMS2 GMP
>
> Important settings:
>   value of $LANG: en_US.UTF-8
>   locale-coding-system: utf-8-unix
>
> Major mode: Image[png]
>
> Minor modes in effect:
>   shell-dirtrack-mode: t
>   tooltip-mode: t
>   global-eldoc-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
>   auto-composition-mode: t
>   auto-encryption-mode: t
>   auto-compression-mode: t
>   line-number-mode: t
>   transient-mark-mode: t
>
> Load-path shadows:
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-gvfs hides 
> /usr/share/emacs/27.1/lisp/net/tramp-gvfs
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-rclone hides 
> /usr/share/emacs/27.1/lisp/net/tramp-rclone
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-compat hides 
> /usr/share/emacs/27.1/lisp/net/tramp-compat
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-loaddefs hides 
> /usr/share/emacs/27.1/lisp/net/tramp-loaddefs
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-ftp hides 
> /usr/share/emacs/27.1/lisp/net/tramp-ftp
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-integration 
> hides /usr/share/emacs/27.1/lisp/net/tramp-integration
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp hides 
> /usr/share/emacs/27.1/lisp/net/tramp
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-sh hides 
> /usr/share/emacs/27.1/lisp/net/tramp-sh
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-cache hides 
> /usr/share/emacs/27.1/lisp/net/tramp-cache
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-adb hides 
> /usr/share/emacs/27.1/lisp/net/tramp-adb
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-archive hides 
> /usr/share/emacs/27.1/lisp/net/tramp-archive
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-cmds hides 
> /usr/share/emacs/27.1/lisp/net/tramp-cmds
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-smb hides 
> /usr/share/emacs/27.1/lisp/net/tramp-smb
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-sudoedit hides 
> /usr/share/emacs/27.1/lisp/net/tramp-sudoedit
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/tramp-uu hides 
> /usr/share/emacs/27.1/lisp/net/tramp-uu
> /home/mikpom/Dropbox/.config/emacs-plugins/tramp/trampver hides 
> /usr/share/emacs/27.1/lisp/net/trampver
>
> Features:
> (shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
> rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
> rmail-loaddefs 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 vc-hg vc-git diff-mode
> vc-bzr help-fns radix-tree cl-print backtrace help-mode find-func
> image-mode easymenu exif tramp-sh noutline outline easy-mmode
> tramp-cache tramp tramp-loaddefs trampver tramp-integration files-x
> tramp-compat shell pcomplete comint ansi-color ring parse-time iso8601
> time-date ls-lisp format-spec auth-source cl-seq eieio eieio-core
> cl-macs eieio-loaddefs cl-loaddefs cl-lib password-cache json subr-x map
> seq byte-opt gv bytecomp byte-compile cconv 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 timer
> select scroll-bar mouse jit-lock font-lock syntax facemenu 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 loaddefs
> button faces cus-face macroexp files 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 emacs)
>
> Memory information:
> ((conses 16 238745 23438)
>  (symbols 48 9577 2)
>  (strings 32 29283 2492)
>  (string-bytes 1 1268698)
>  (vectors 16 23567)
>  (vector-slots 8 1083940 157766)
>  (floats 8 55 178)
>  (intervals 56 80877 0)
>  (buffers 1000 14))



[-- Attachment #1.2: Type: text/html, Size: 9259 bytes --]

[-- Attachment #2: files.tar.gz --]
[-- Type: application/gzip, Size: 385134 bytes --]

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Michael Albinus]

Mikhail P <mikpom@fastmail.com> writes:

> Greetings.

Hi,

> I am experiencing problems with viewing remote images. When resizing
> an image (e.g. by resizing Emacs frame) Emacs occasionally
> (approximately ~30% reproducibility) can prompt ... changed on disk;
> really edit the buffer? which does not make a lot of sense. Regardless
> of my response Emacs outputs errors, lags for several seconds and only
> after that resizes the image.
>
> I am using currently most recent commit on TRAMP repo
> (69844458e33b5dcae53de249d9d82c59a5876055) containing some necessary
> fixes. Prior to these fixes Emacs could occasionally stall on resizing
> remote image and only interruption with C-g could help (or sometimes
> making Emacs completely unresponsive). Also attaching TRAMP debug file
> which I hope exposes the problem.

FTR, this has been merged with the Emacs git master.

> As far as I could understand from my conversation with TRAMP devs
> while working on the fixes there are problems with functions called on
> idle-timer in image mode ... 😬

Well, we see in the debug file the following backtrace:

--8<---------------cut here---------------start------------->8---
  backtrace()
  tramp-send-string((tramp-file-name "ssh" nil nil "horsehop" nil "/storage1/mikpom/wizard_devel/CNV/figs/TCGA_CNV/AT..." nil) "(env QUOTING_STYLE=locale \\stat -c '((/////%N/////...")
  tramp-send-command((tramp-file-name "ssh" nil nil "horsehop" nil "/storage1/mikpom/wizard_devel/CNV/figs/TCGA_CNV/AT..." nil) "(env QUOTING_STYLE=locale \\stat -c '((/////%N/////...")

[...]

  tramp-file-name-handler(file-readable-p "/ssh:horsehop:/storage1/mikpom/wizard_devel/CNV/fi...")
  file-readable-p("/ssh:horsehop:/storage1/mikpom/wizard_devel/CNV/fi...")
  image-toggle-display-image()
  image-fit-to-window(#<window 3 on TCGA-02-2485.png>)
  apply(image-fit-to-window #<window 3 on TCGA-02-2485.png>)
  timer-event-handler([t 0 1 0 nil image-fit-to-window (#<window 3 on TCGA-02-2485.png>) idle 0])

[...]

  tramp-file-name-handler(verify-visited-file-modtime #<buffer TCGA-02-2485.png>)
  add-text-properties(1 64075 (display (image :type png :data "\211PNG\15\n\32\n\0\0\0\15IHDR\0\0\5\0\0\0\3\300\10\6\0\0\0j\334\324y\0\0\0\4sBIT\10\10\10\10|\10d\210\0..." :scale 1 :max-width 1152 :max-height 884) rear-nonsticky (display) read-only t front-sticky (read-only)))
  image-toggle-display-image()
  image-fit-to-window(#<window 3 on TCGA-02-2485.png>)
  apply(image-fit-to-window #<window 3 on TCGA-02-2485.png>)
  timer-event-handler([t 0 1 0 nil image-fit-to-window (#<window 3 on TCGA-02-2485.png>) idle 0])
18:56:54.633834 tramp-send-string (1) # Remote file error (compat): Forbidden reentrant call of Tramp
--8<---------------cut here---------------end--------------->8---

That is, two calls of timer-event-handler([t 0 1 0 nil image-fit-to-window ...])

image-fit-to-window calls remote file operations,
verify-visited-file-modtime the first time, file-readable-p the second
time. The first call of image-fit-to-window hasn't finished, when the
second call of image-fit-to-window happens one second later. That is
relevant, this situation happens only for slow remote connections (I had
a hard time to reconstruct the scenario).

From my pov, image-fit-to-window must be hardened in order to avoid this
reentrant call. Maybe an internal lock at entry, which is honored by
every next call until the lock is removed.

And at least it shall be wrapped with

(ignore-error 'remote-file-error ...)

in order to masque the situation.

> Thanks,
>
> Mikhail

Best regards, Michael.

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Juri Linkov]

> That is, two calls of timer-event-handler([t 0 1 0 nil image-fit-to-window ...])
>
> image-fit-to-window calls remote file operations,
> verify-visited-file-modtime the first time, file-readable-p the second
> time. The first call of image-fit-to-window hasn't finished, when the
> second call of image-fit-to-window happens one second later. That is
> relevant, this situation happens only for slow remote connections (I had
> a hard time to reconstruct the scenario).
>
> From my pov, image-fit-to-window must be hardened in order to avoid this
> reentrant call. Maybe an internal lock at entry, which is honored by
> every next call until the lock is removed.

The simplest solution is just to increase the number of seconds
in the user option 'image-auto-resize-on-window-resize'
proportionally to network latency.

Locking could be implemented as well.  How would be better to do this?
Maybe by using a buffer-local variable?

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Michael Albinus]

Juri Linkov <juri@linkov.net> writes:

Hi Juri,

>> From my pov, image-fit-to-window must be hardened in order to avoid this
>> reentrant call. Maybe an internal lock at entry, which is honored by
>> every next call until the lock is removed.
>
> The simplest solution is just to increase the number of seconds
> in the user option 'image-auto-resize-on-window-resize'
> proportionally to network latency.

That's a global variable, right? So you would also delay resizing of
images located locally.

> Locking could be implemented as well.  How would be better to do this?
> Maybe by using a buffer-local variable?

Yes. Something like

--8<---------------cut here---------------start------------->8---
(defvar image-fit-to-window-lock nil
  "Lock for `image-fit-to-window' timer."

(defun image-fit-to-window (window)
  "..."
  (unless image-fit-to-window-lock
    (unwind-protect
	(progn
	  (setq-local image-fit-to-window-lock t)
	  ...)
      (setq image-fit-to-window-lock nil))))
--8<---------------cut here---------------end--------------->8---

There's also another thread. When image-fit-to-window is called from
Emacs the first time, there could also be a running Tramp operation,
which would be disturbed. See the recent discussion about "Tramp and
timers" in the emacs-devel ML. Tramp would detect this situation, and
fire the (new) error remote-file-error. This must also be handled, like

--8<---------------cut here---------------start------------->8---
(defun image-fit-to-window (window)
  "..."
  (ignore-error 'remote-file-error
    ...))
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Juri Linkov]

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

>> The simplest solution is just to increase the number of seconds
>> in the user option 'image-auto-resize-on-window-resize'
>> proportionally to network latency.
>
> That's a global variable, right? So you would also delay resizing of
> images located locally.

Right, but disabling image-auto-resize-on-window-resize to nil in init file
will help as a temporary measure until this problem is properly fixed
for slow connections.

>> Locking could be implemented as well.  How would be better to do this?
>> Maybe by using a buffer-local variable?
>
> Yes. Something like
>
> (defvar image-fit-to-window-lock nil
>   "Lock for `image-fit-to-window' timer."
>
> (defun image-fit-to-window (window)
>   "..."
>   (unless image-fit-to-window-lock
>     (unwind-protect
> 	(progn
> 	  (setq-local image-fit-to-window-lock t)
> 	  ...)
>       (setq image-fit-to-window-lock nil))))
>
>
> There's also another thread. When image-fit-to-window is called from
> Emacs the first time, there could also be a running Tramp operation,
> which would be disturbed. See the recent discussion about "Tramp and
> timers" in the emacs-devel ML. Tramp would detect this situation, and
> fire the (new) error remote-file-error. This must also be handled, like
>
> (defun image-fit-to-window (window)
>   "..."
>   (ignore-error 'remote-file-error
>     ...))

Additionally to these changes, I also added canceling the previous timer
before starting a new timer to avoid several simultaneously started timers.
This improved responsiveness in non-remote case.  In remote case it helps
a little too, but still needs the lock for extremely slow connections.
Here is a complete patch (BTW, I'm not sure if the check for file-remote-p
can be removed now):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: image-fit-to-window-lock.patch --]
[-- Type: text/x-diff, Size: 2478 bytes --]

diff --git a/lisp/image-mode.el b/lisp/image-mode.el
index 032ebf3873..a5b610745c 100644
--- a/lisp/image-mode.el
+++ b/lisp/image-mode.el
@@ -795,7 +795,7 @@ image-toggle-display-image
   (let* ((filename (buffer-file-name))
 	 (data-p (not (and filename
 			   (file-readable-p filename)
-			   (not (file-remote-p filename))
+			   (not (file-remote-p filename)) ; TODO: remove?
 			   (not (buffer-modified-p))
 			   (not (and (boundp 'archive-superior-buffer)
 				     archive-superior-buffer))
@@ -942,6 +942,9 @@ image-after-revert-hook
           (get-buffer-window-list (current-buffer) 'nomini 'visible))
     (image-toggle-display-image)))
 
+(defvar image-auto-resize-timer nil
+  "Timer for `image-auto-resize-on-window-resize' option.")
+
 (defun image--window-state-change (window)
   ;; Wait for a bit of idle-time before actually performing the change,
   ;; so as to batch together sequences of closely consecutive size changes.
@@ -950,8 +953,15 @@ image--window-state-change
   ;; consecutive calls happen without any redisplay between them,
   ;; the costly operation of image resizing should happen only once.
   (when (numberp image-auto-resize-on-window-resize)
-    (run-with-idle-timer image-auto-resize-on-window-resize nil
-                         #'image-fit-to-window window)))
+    (when image-auto-resize-timer
+      (cancel-timer image-auto-resize-timer)
+      (setq image-auto-resize-timer nil))
+    (setq image-auto-resize-timer
+          (run-with-idle-timer image-auto-resize-on-window-resize nil
+                               #'image-fit-to-window window))))
+
+(defvar image-fit-to-window-lock nil
+  "Lock for `image-fit-to-window' timer function.")
 
 (defun image-fit-to-window (window)
   "Adjust size of image to display it exactly in WINDOW boundaries."
@@ -968,7 +978,13 @@ image-fit-to-window
               (when (and image-width image-height
                          (or (not (= image-width  window-width))
                              (not (= image-height window-height))))
-                (image-toggle-display-image)))))))))
+                (unless image-fit-to-window-lock
+                  (unwind-protect
+                      (progn
+                        (setq-local image-fit-to-window-lock t)
+                        (ignore-error 'remote-file-error
+                          (image-toggle-display-image)))
+                    (setq image-fit-to-window-lock nil)))))))))))
 
 \f
 ;;; Animated images

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Michael Albinus]

Juri Linkov <juri@linkov.net> writes:

Hi Juri,

> Additionally to these changes, I also added canceling the previous timer
> before starting a new timer to avoid several simultaneously started timers.
> This improved responsiveness in non-remote case.  In remote case it helps
> a little too, but still needs the lock for extremely slow connections.

Why cancelling the *previous* timer? It has done already part of the
job, so I would expect it will finish faster than a newly started timer.

> Here is a complete patch (BTW, I'm not sure if the check for file-remote-p
> can be removed now):

Yes. file-readable-p has performed already a remote operation (it runs
"test -r <filename>"), so a test file-remote-p doesn't make sense.

You haven't protected the timer function against the
remote-file-error. Are you sure it cannot happen?

Best regards, Michael.

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Juri Linkov]

>> Additionally to these changes, I also added canceling the previous timer
>> before starting a new timer to avoid several simultaneously started timers.
>> This improved responsiveness in non-remote case.  In remote case it helps
>> a little too, but still needs the lock for extremely slow connections.
>
> Why cancelling the *previous* timer? It has done already part of the
> job, so I would expect it will finish faster than a newly started timer.

Actually, the previous timer has not done any job, it's just waiting
for idle time to call the timer function that will do the job.
If a previous timer function (not timer) is stuck in file-readable-p,
this is a separate case for slow connections that is handled
by the lock in the timer function.

>> Here is a complete patch (BTW, I'm not sure if the check for file-remote-p
>> can be removed now):
>
> Yes. file-readable-p has performed already a remote operation (it runs
> "test -r <filename>"), so a test file-remote-p doesn't make sense.

But maybe using the image data from the buffer is much faster to display
the image (image data was already inserted to the image buffer by previous
remote file-reading operation), than to read the image data from the
remote file again by internal image-displaying functions.

> You haven't protected the timer function against the
> remote-file-error. Are you sure it cannot happen?

There is no need to protect the whole timer function
that doesn't use remote calls, so I protected only the
image-toggle-display-image call in the timer function.

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Michael Albinus]

Juri Linkov <juri@linkov.net> writes:

Hi Juri,

> But maybe using the image data from the buffer is much faster to display
> the image (image data was already inserted to the image buffer by previous
> remote file-reading operation), than to read the image data from the
> remote file again by internal image-displaying functions.

Yes, this sounds smoother.

>> You haven't protected the timer function against the
>> remote-file-error. Are you sure it cannot happen?
>
> There is no need to protect the whole timer function
> that doesn't use remote calls, so I protected only the
> image-toggle-display-image call in the timer function.

I see. Thanks.

Best regards, Michael.

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Juri Linkov]

tags 45256 fixed
close 45256 28.0.50
quit

>> There is no need to protect the whole timer function
>> that doesn't use remote calls, so I protected only the
>> image-toggle-display-image call in the timer function.
>
> I see. Thanks.

So I pushed the patch to master, and closed this bug report.

bug#45256: Viewing images over network using TRAMP (errors and unexpected prompts)

[Mikhail Pomaznoy]

Tried out the latest version from git repo and it fixes the bug in my 
setting.

Thanks,

Mikhail

On 20.12.2020 03:19, Juri Linkov wrote:
> tags 45256 fixed
> close 45256 28.0.50
> quit
>
>>> There is no need to protect the whole timer function
>>> that doesn't use remote calls, so I protected only the
>>> image-toggle-display-image call in the timer function.
>> I see. Thanks.
> So I pushed the patch to master, and closed this bug report.