Auto-revert on remote files: disabling timers temporarily?

Auto-revert on remote files: disabling timers temporarily?

[Michael Albinus]

In parallel to Kai, I've tried to find another solution. What about
disabling timers during execution of a Tramp file name function?
Something like this:

(defun tramp-file-name-handler (operation &rest args)
  "Invoke Tramp file name handler.
Falls back to normal file name handler if no tramp file name handler exists."
  (save-match-data
    (let* ((filename (apply 'tramp-file-name-for-operation operation args))
	   (foreign (tramp-find-foreign-file-name-handler filename)))
      (cond
       (foreign
	;; Tramp functions called by timers would disturb process
	;; communication.  Therefore, timers are disabled temporarily.
	(let ((tl timer-list))
	  (unwind-protect
	      (progn
		(setq timer-list nil)
		(apply foreign operation args))
	    (setq timer-list tl))))
       (t (tramp-run-real-handler operation args))))))

Locally, it works fine for me, even with enabled auto-revert-mode for
remote files. But I have no idea about side effects. Are there obvious
objections against?

Best regards, Michael.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Michael Albinus wrote:

   In parallel to Kai, I've tried to find another solution. What about
   disabling timers during execution of a Tramp file name function?

With what intent?

Only to restore auto-reverting of remote files?  If so, would that
mainly be to help debug other problems or because there would appear
to be a real need to revert remote files?  Temporarily re-enabling
auto-reverting of remote files for debugging purposes can easily be
done by erasing one line in ` auto-revert-handler', namely:
(not (file-remote-p buffer-file-name))

On my connection a call to `file-readable-p' for a remote file takes
approximately .35 to .4 seconds.  It can take substantially longer
under less ideal conditions.  (For a local file it takes
substantially less than .001 second, it is essentially unmeasurable.)

With fifteen remote files and global-auto-revert-mode enabled, and the
default value of auto-revert-interval (five seconds)
auto-revert-buffers would essentially be running `file-readable-p' on
remote files constantly, disabling other timers.  This assuming that
no files ever would ever actually need to be reverted.

Actually reverting depends a lot on the size of the file.  For a
moderately sized file like streams.texi, it takes more than 15
seconds.  Again, it can take much longer under less than ideal
conditions.  This assumes that all the connection setup has already
been done.

`auto-revert-stop-on-user-input' only works _between_ taking care of
individual buffers.  So user input could be disabled for more than
fifteen seconds, sometimes _much_ more than that.  One could use
`with-local-quit' to allow interrupting with C-g, but the trouble
starts again 0 to 5 seconds later, so that is no big help.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

>From my earlier message:

   With fifteen remote files and global-auto-revert-mode enabled, and the
   default value of auto-revert-interval (five seconds)
   auto-revert-buffers would essentially be running `file-readable-p' on
   remote files constantly, disabling other timers.

I forgot that `auto-revert-handler' also calls
`verify-visited-file-modtime', which takes, on my connection, .43 to
.55 seconds a call for remote files, so with nearly one second for
checking each remote file, under ideal conditions (for my connection),
things seem hopeless.  If any remote file actually needs to be
reverted, trouble can ensue.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

All numbers I gave assume that the connection is already set up.
Otherwise the numbers are tremendously worse.  For instance,
file-readable-p takes, on my connection, more than 20 seconds, _after_
entering my password. for remote files, if the connection still needs
to be opened.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Michael Albinus wrote:
>
>    In parallel to Kai, I've tried to find another solution. What about
>    disabling timers during execution of a Tramp file name function?
>
> With what intent?
>
> Only to restore auto-reverting of remote files?  If so, would that
> mainly be to help debug other problems or because there would appear
> to be a real need to revert remote files?

It's not for debugging. If you have a fair good link to the remote
host(s), auto-reverting works for remote files as well. But it must be
avoided that auto-reverting interrupts running Tramp actions.

Of course, it doesn't make any sense for slow links. So there should
be an option enabling/disabling auto-reverting for remote files, as
proposed by Kai.

> Sincerely,
>
> Luc.

Best regards, Michael.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Michael Albinus wrote:

   Of course, it doesn't make any sense for slow links. So there should
   be an option enabling/disabling auto-reverting for remote files, as
   proposed by Kai.

That would be trivial to do.  I believe it would be OK if the default
were to disable and if the docstring gave suitable warning that
enabling can cause problems on slow connections.  If we go for an
option, then another question is whether autoreverting of remote
directories is sufficiently fast on fast connections that, if both
this option, say global-auto-revert-remote-files, and
global-auto-revert-non-file-buffers are enabled, then remote
directories should autorevert as well.

Note that, if I enable autoreverting of remote files, then, even with
the new Tramp version, I still get either a hang or an "Invalid base64
data" error if I try to visit my second remote file using Tramp-ssh.
I have a slow connection.  I do not get crashes any longer, however.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Michael Albinus wrote:
>
>    Of course, it doesn't make any sense for slow links. So there should
>    be an option enabling/disabling auto-reverting for remote files, as
>    proposed by Kai.
>
> That would be trivial to do.  I believe it would be OK if the default
> were to disable and if the docstring gave suitable warning that
> enabling can cause problems on slow connections.

Sounds OK to me.

> If we go for an option, then another question is whether
> autoreverting of remote directories is sufficiently fast on fast
> connections that, if both this option, say
> global-auto-revert-remote-files, and
> global-auto-revert-non-file-buffers are enabled, then remote
> directories should autorevert as well.

It depends. Nevertheless, with my proposed patch (disabling timers
temporarily) it would be handled as well: If auto-reverting of a
directory lasts more than 5 sec, next time it should occur the timer
is disabled. So it happens in periods of 10, or 15, or whatever
seconds. If it lasts too long, the user should disable it.

Maybe there should be an option for the time period auto-reverting is
acting on remote files/directories? In your case (slow conmnection),
every minute would be an appropriate value. In other cases, 10 sec or
even the todays 5 sec would be sufficient.

> Note that, if I enable autoreverting of remote files, then, even with
> the new Tramp version, I still get either a hang or an "Invalid base64
> data" error if I try to visit my second remote file using Tramp-ssh.
> I have a slow connection.  I do not get crashes any longer, however.

That's definitely a bug. It would be great if you could produce a
backtrace, and if you could provide involved Tramp buffers.

> Sincerely,
>
> Luc.

Best regards, Michael.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Michael Albinus wrote:

   > Note that, if I enable autoreverting of remote files, then, even with
   > the new Tramp version, I still get either a hang or an "Invalid base64
   > data" error if I try to visit my second remote file using Tramp-ssh.
   > I have a slow connection.  I do not get crashes any longer, however.

   That's definitely a bug. It would be great if you could produce a
   backtrace, and if you could provide involved Tramp buffers.

Here is a backtrace for the hang (the usual behavior).  I will give a
backtace for the error if I can still reproduce it.  It happens
infrequently.  There _always_ is either a hang or an error however.
The Tramp debug buffer gets messed up by printing auto-revert related
stuff every five seconds.

Debugger entered--Lisp error: (quit)
  accept-process-output(#<process *tramp/ssh raven.dms.auburn.edu*> 1)
  tramp-wait-for-output()
  tramp-send-command-and-check(nil "ssh" nil "raven.dms.auburn.edu" "mimencode -b < /home/teirllm/strings.texi" nil)
  tramp-barf-unless-okay(nil "ssh" nil "raven.dms.auburn.edu" "mimencode -b < /home/teirllm/strings.texi" nil file-error "Encoding remote file failed, see buffer `%s' for details" #<buffer *tramp/ssh raven.dms.auburn.edu*>)
  tramp-handle-file-local-copy("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  apply(tramp-handle-file-local-copy "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  tramp-sh-file-name-handler(file-local-copy "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  apply(tramp-sh-file-name-handler file-local-copy "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  tramp-file-name-handler(file-local-copy "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  file-local-copy("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  tramp-handle-insert-file-contents("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil nil)
  apply(tramp-handle-insert-file-contents ("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil nil))
  tramp-sh-file-name-handler(insert-file-contents "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil nil)
  apply(tramp-sh-file-name-handler insert-file-contents ("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil nil))
  tramp-file-name-handler(insert-file-contents "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil nil)
  insert-file-contents("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t)
  \"byte-									      code("[inhibit-read-only filename t insert-file-contents] 3)
  find-file-noselect-1(#<buffer strings.texi> "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" nil nil "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" ((38 . 25870) (-1 0)))
  find-file-noselect("/ssh:raven.dms.auburn.edu:strings.texi" nil nil t)
  find-file("/ssh:raven.dms.auburn.edu:strings.texi" t)
  call-interactively(find-file)

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Here is a backtrace for the error:

Debugger entered--Lisp error: (error "Invalid base64 data")
  base64-decode-region(1 21)
  tramp-handle-file-local-copy("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  apply(tramp-handle-file-local-copy
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  tramp-sh-file-name-handler(file-local-copy
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  apply(tramp-sh-file-name-handler file-local-copy
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  tramp-file-name-handler(file-local-copy
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  file-local-copy("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")
  tramp-handle-insert-file-contents("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi"
  t nil nil nil)
  apply(tramp-handle-insert-file-contents
  ("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil
  nil))
  tramp-sh-file-name-handler(insert-file-contents
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil
  nil)
  apply(tramp-sh-file-name-handler insert-file-contents
  ("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil
  nil))
  tramp-file-name-handler(insert-file-contents
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" t nil nil
  nil)
  insert-file-contents("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi"
  t)
  byte-code   [inhibit-read-only filename t insert-file-contents] 3)
  find-file-noselect-1(#<buffer strings.texi<2>>
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" nil nil
  "/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi" ((38 . 25870)
  (-1 0)))
  find-file-noselect("/ssh:raven.dms.auburn.edu:strings.texi" nil nil
  t)
  find-file("/ssh:raven.dms.auburn.edu:strings.texi" t)
  call-interactively(find-file)

Re: Auto-revert on remote files: disabling timers temporarily?

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Here is a backtrace for the hang (the usual behavior).  I will give a

The backtrace doesn't look exciting. Normal Tramp actions I would say.

> backtace for the error if I can still reproduce it.  It happens
> infrequently.  There _always_ is either a hang or an error however.
> The Tramp debug buffer gets messed up by printing auto-revert related
> stuff every five seconds.

I guess you have the recent Emacs CVS, including Tramp 2.0.41 and
Kai's post-release fix? The auto-revert stuff shouldn't appear during
Tramp actions ...

Both the debug and the communication buffers, even if large, would be helpfull.

Best regards, Michael.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Michael Albinus wrote:

   I guess you have the recent Emacs CVS, including Tramp 2.0.41 and
   Kai's post-release fix? 

Yes.

			   The auto-revert stuff shouldn't appear during
   Tramp actions ...

No, but auto-revert keeps invoking Tramp countless times immediately after
the quit or error.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Here is a backtrace for the error:
>
> Debugger entered--Lisp error: (error "Invalid base64 data")
>   base64-decode-region(1 21)
>   tramp-handle-file-local-copy("/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi")

Very likely the data in the Tramp buffer has been changed between
base64 encoding/decoding. It would be important to see the related
Tramp buffers.

Btw, you could work around this problem using scp instead of ssh I guess.

Best regards, Michael.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Now I all of a sudden _always_ get a hang out of which I can _not_
quit.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Michael Albinus wrote:

   Very likely the data in the Tramp buffer has been changed between
   base64 encoding/decoding. It would be important to see the related
   Tramp buffers.

I have trouble reproducing the error again.  Now I keep getting hangs
out of which I can not quit.  Nothing changed, except maybe the
quality of my connection and even that did not change noticeably.

I did save the only part of the Tramp debug buffer which I believed
could be relevant.  It does not seem very informative.  The Tramp
process buffer always seems to contain only the latest line. 

tramp_exit_status 0
# Decoding remote file
/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi...
# Decoding remote file
/ssh:raven.dms.auburn.edu:/home/teirllm/strings.texi with function
base64-decode-region...

This is where the error occurred.  Now we start auto-reverting again.
So it would not seem useful to go through much effort trying to
reproduce the error again, since the Debug buffer seems to give no
useful info.

$ test -r /home/teirllm/sequences.texi 2>/dev/null; echo
tramp_exit_status $? 
tramp_exit_status 0
$ test -e /home/teirllm/sequences.texi 2>/dev/null; echo
tramp_exit_status $? 
tramp_exit_status 0
$ tramp_file_attributes /home/teirllm/sequences.texi nil
(nil 1 125 103 (16571 39836) (16547 43368) (16547 43368) 23830 33152 t
(38 . 25873) -1)

Re: Auto-revert on remote files: disabling timers temporarily?

[Kim F. Storm]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Now I all of a sudden _always_ get a hang out of which I can _not_
> quit.

I don't know if it is relevant in your case, but try to undo the
last change in process.c (use rev 1.429) and see if that helps.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Kim Storm wrote:

   Luc Teirlinck <teirllm@dms.auburn.edu> writes:

   > Now I all of a sudden _always_ get a hang out of which I can _not_
   > quit.

   I don't know if it is relevant in your case, but try to undo the
   last change in process.c (use rev 1.429) and see if that helps.

That seems to solve the problems.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

My previous reply was premature.  Justv ignore it for the time being.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Kim Storm wrote:

   Luc Teirlinck <teirllm@dms.auburn.edu> writes:

   > Now I all of a sudden _always_ get a hang out of which I can _not_
   > quit.

   I don't know if it is relevant in your case, but try to undo the
   last change in process.c (use rev 1.429) and see if that helps.

I still get the quittable hangs and the "Invalid base64 data" errors,
just after loading more (than one) remote files.  The fact that I can
load more remote files and do, for the moment get no non-quittable
hangs might be due to coincidence ot to a temporarily better
connection, however.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

>From my previous message:

   I still get the quittable hangs and the "Invalid base64 data" errors,
   just after loading more (than one) remote files.  The fact that I can
   load more remote files and do, for the moment get no non-quittable
   hangs might be due to coincidence ot to a temporarily better
   connection, however.

I might accidentally have done something strange in that one session.
In all other tries, the problem starts again immediately with the
second remote file.  I have not seen the unquittable hangs again, just
quittable hangs and errors, but that probably is a coincidence too.

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Luc Teirlinck]

Michael Albinus wrote:

   I guess you have the recent Emacs CVS, including Tramp 2.0.41 and
   Kai's post-release fix? The auto-revert stuff shouldn't appear during
   Tramp actions ...

I have the version of Tramp from the latest _Emacs_ CVS, including
Andreas' fix to the bug I reported (the last change to Tramp in Emacs
CVS).  However, to avoid confusion, I did not apply the patch you
proposed.  (Disabling all timers during execution of Tramp).

Sincerely,

Luc.

Re: Auto-revert on remote files: disabling timers temporarily?

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Here is the Tramp debug buffer after the latest "Invalid base64 data".

It's pretty clear now what happens. A proper encode/decode sequence must
look like this:

> # Encoding remote file /ssh:raven.dms.auburn.edu:/home/teirllm/sequences.texi...
> $ mimencode -b < /home/teirllm/sequences.texi 2>/dev/null; echo tramp_exit_status $? 
> QGMgLSotdGV4aW5mby0qLQpAYyBUaGlzIGlzIHBhcnQgb2YgdGhlIEdOVSBFbWFjcyBMaXNw
[...]
> Mzg2CkBlbmQgaWdub3JlCg==
> tramp_exit_status 0
> # Decoding remote file /ssh:raven.dms.auburn.edu:/home/teirllm/sequences.texi...
> # Decoding remote file /ssh:raven.dms.auburn.edu:/home/teirllm/sequences.texi with function base64-decode-region...
> # Decoding remote file /ssh:raven.dms.auburn.edu:/home/teirllm/sequences.texi...done

But later, there is such a sequence:

> # Encoding remote file /ssh:raven.dms.auburn.edu:/home/teirllm/streams.texi...
> $ mimencode -b < /home/teirllm/streams.texi 2>/dev/null; echo tramp_exit_status $? 
> $ test -r /home/teirllm/strings.texi 2>/dev/null; echo tramp_exit_status $? 
> IGFuZCB0aGUgQHNhbXB7aH0gaW4KdGhlIHdvcmQgQHNhbXB7dGhlfS4gIEF0IHRoZSBlbmQs
[...]
> My00NzM1LTllMDYtMmU4NjQzMjBiNDM0CkBlbmQgaWdub3JlCg==
> tramp_exit_status 0
> $ test -e /home/teirllm/strings.texi 2>/dev/null; echo tramp_exit_status $? 
> tramp_exit_status 0
> $ tramp_file_attributes /home/teirllm/strings.texi nil
> tramp_exit_status 0
> tramp_exit_status 0
> (nil 1 125 103 (16571 47596) (16313 37121) (16313 37121) 39315 33152 t (38 . 25870) -1)
> # Decoding remote file /ssh:raven.dms.auburn.edu:/home/teirllm/streams.texi...
> # Decoding remote file /ssh:raven.dms.auburn.edu:/home/teirllm/streams.texi with function base64-decode-region...

You see the famous "test -r", "test -e" and "tramp_file_attributes"
(coming from auto-revert) there.

Oops, I'm too stupid: Kai's patch locking Tramp actions didn't reach
Emacs CVS yet. Could you, please, try tramp.el from Tramp CVS? You
should use the HEAD version, it isn't synced yet with tramp-stable
with respect to this patch.

Best regards, Michael.