unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* TRAMP VC optimization fails: non-TRAMP filenames handled incorrectly in async operations.
@ 2019-03-27 15:38 Daniel Pittman
  2019-03-27 15:55 ` Michael Albinus
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Pittman @ 2019-03-27 15:38 UTC (permalink / raw)
  To: emacs-devel

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

G'day.  In HEAD, tramp-sh has some optimizations in place for the
`vc-registered` operation, intended to minimize the number of round trips.
Sadly, these don't seem to be safe in a single threaded Emacs world, and
even less safe in a threaded one.

The function `tramp-sh-handle-vc-registered` attempts to optimize the
process by emplacing a separate file-name-handler, running the
operation(s), then performing a single remote call to obtain all the data.

This is an optimization, no doubt, and would be great ... except that it
assumes a synchronous operation where no other code can run while the new
file handler is in effect.  This is, sadly, not the case.

The observable symptom is that we hit this error throw in
`tramp-dissect-file-name`:
(tramp-user-error nil "Not a Tramp file name: \"%s\"" name)

The most common place I observed this was during startup, when it
complained that the `nsm-settings-file`, which is the default value in my
case, was not a TRAMP filename.  It also happens for expanding `~`, and a
handful of other cases, when I was reverting tramp buffers, etc.

On debugging I discovered that the problems were springing from calls
something akin to this:
(let ((default-directory "/ssh:slippycheeze@example.com:/tmp"))
  (expand-file-name "~" default-directory))

That uses, via `tramp-vc-file-name-handler`,
`tramp-file-name-for-operation`, which in the case of `expand-file-name`
returns "~".  I believe that is correct, but if not, that may be the root
cause of the problem.

In any case, that is then passed to `tramp-dissect-file-name` by
`tramp-vc-file-name-handler`, and (definitely correctly) triggers the error
that a non-tramp filename is being parsed.

This triggered from an async lisp callback firing, while the current buffer
happened to be on tramp.  The previously mentioned `nsm-settings-file` is
referenced when an async update of ELPA package lists – triggered in my
init.el file – is running, and desktop.el is busy reloading my buffers,
including the ones via tramp.

So, basically, the optimization would work great if only we never called
any lisp code asynchronously to it – but we do, and that is probably made
more visible when I'm using an ssh-backed method to talk to a machine ~
250ms away from my Emacs instance than when it is ~ 25ms away.  A bigger
race, where TRAMP is waiting for data from an external process, and so
other external processes can also fire.

I don't have a patch, but I'll see if I can figure out how to improve
this.  I fear the situation is impossible, however, and that this attempt
to improve the performance of `vc-registered` is doomed to failure unless
an async protocol is defined to replace the current, synchronous, version.

I also cannot say if the problem extends beyond `expand-file-name`, but it
probably hits at minimum anything else that canonicalizes paths.

[-- Attachment #2: Type: text/html, Size: 3409 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-04-02 13:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-27 15:38 TRAMP VC optimization fails: non-TRAMP filenames handled incorrectly in async operations Daniel Pittman
2019-03-27 15:55 ` Michael Albinus
2019-03-27 16:22   ` Daniel Pittman
2019-03-27 17:49     ` Michael Albinus
2019-03-29 12:36       ` Daniel Pittman
2019-03-29 17:03         ` Michael Albinus
2019-04-02  9:34           ` Daniel Pittman
2019-04-02 13:18             ` Michael Albinus

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).