bug#61350: Eglot over Tramp freezes with large project

[João Távora]

Thomas Koch <thomas@koch.ro> writes:

> Thank you both for being so constructively engaged in the bug I opened.
>
> I'll give Joao's patch a try tomorrow. (I'm too tired now.) However I don't 
> see ControlMaster as the root cause at the moment.
>
> My current working theory is:
>
> 1. There is some buffer in SSH (or TCP) that gets filled by the language 
> server sending data to eglot.
> 2. Tramp sends a command over SSH.
> 3. Tramp sets the JUST-THIS-ONE option of accept-process-output to t since 
> https://debbugs.gnu.org/12145.
> 4. The response to 2. can not arrive due to the buffer being filled in 1. 
> Tramp blocks the emacs main thread.

> I tested my theory by deleting (and thus disabling) the JUST-THIS-ONE
> argument in all calls to accept-process-output in tramp.el and
> tramp-sh.el. Eglot did not freeze anymore

I don't think your observation necessarily validates your theory.  If in
'1.' you mean _an Emacs buffer_, the buffer in question is populated by
Tramp's process filter (the default process filter, that is).  Then
accept-process-output with non-nil JUST-THIS-ONE would not prevent that
buffer from getting the all the output, becasue the "THIS-ONE" is the
process.

But JUST-THIS-ONE _is_ relevant when there is more than one process.
Here, there is.  There's one process, the jsonrpc.el process, henceforth
'jprocess', and the Tramp process, henceforth 'tprocess'.  jprocess
receives only JSONRPC data from the LSP server.  It "thinks" it is
talking directly to a JSONRPC server, but in Tramp scenarios it is being
fed data from tprocess, which is the process connected to the remote
host.  In tprocess, other things, such as shell interactions are going
on.

Michael can probably confirm, correct or deny this.

When one (accept-process-output tprocess nil nil 'JUST-THIS-ONE=t) one
must be absolutely sure that tprocess is going to send _something_
"soon".  If it doesn't, we'll hang indefinitely (until the process dies
or the user quits)

That's what has been confirmed through a backtrace.  It's a particular
accept-process-output call in tramp-wait-for-regexp that hangs,
understandibly so.

The need to ensure safety of that call is why there is a prior for any
'found' messages in 'tramp-wait-for-regexp'.  Only then is the "risky"
'accept-process-output' attempted.

(defun tramp-wait-for-regexp (proc timeout regexp)
  ...
  (let ((found (tramp-check-for-regexp proc regexp)))
    (cond (...)
          (t
           (while (not found)
             (tramp-accept-process-output proc) ;; WE "HANG" HERE SOMETIMES
             (unless (process-live-p proc)
               (tramp-error-with-buffer
                nil proc 'file-error "Process has died"))
             (setq found (tramp-check-for-regexp proc regexp)))))


'found' relies on searching the contents of what is already in
tprocess's buffer, which tprocess's filter dumped there.  So far so
good.

Now, 'tramp-check-for-regexp' uses a somewhat non-standard technique of
searching for messages: it searches them from the back, from the end of
the tprocess's buffer.  I don't know what motivated this, but I find it
odd.  I find one of its callees, tramp-search-regexp, particularly
suspicious:

   (defun tramp-search-regexp (regexp)
     "Search for REGEXP backwards, starting at point-max.""
     (goto-char (point-max))
     ;; We restrict ourselves to the last 256 characters.  There were
     ;; reports of a shell command "git ls-files -zco --exclude-standard"
     ;; with 85k files involved, which has blocked Tramp forever.
     (re-search-backward regexp (max (point-min) (- (point) 256)) 'noerror))

See the comment there?  Only 256 characters back are inspected.

So, finally, here's my conjecture:

1. Tramp goes into 'tramp-wait-for-regexp'.  tprocess's buffer already
   the message that 'found' is supposed to return, but it also has a lot
   more stuff, say a lot of JSONRPC data from the LSP server that also
   came into that tprocess buffer and is awaiting to be delivered to
   jprocess.

2. This data is for piping into jprocess, where the JSONRPC message will
   be decoded, but it will probably never arrive at its destination.

3. 'found' will be nil in tramp-wait-for-regexp, because of the
   tramp-search-regexp limitation.

4. tramp-wait-for-regexp will issue the "risky" accept-process-output
   call.

5. there is no more data that accept-process-output wants to put in the
   buffer,  because the LSP server is fine for the moment.

6. Emacs hang

Just a conjecture.

More comments to your comments.

> Jsonrpc needs its own markers because N messages can arrive at any
> given time and the buffer can not be deleted after each message.

Jsonrpc operates in jprocess's buffer, which is separate.

> I have a vague feeling, that Tramp could be improved with a work queue
> such that requests to tramp from notification or timer threads get
> blocked while another tramp command is still waiting for a
> reply.

There are no (usable) threads in Emacs.  Timers are events, and so are
runs of each processe's process filter.  Those two are what creates
asynchronicity and the emulation of simultaneity in Emacs.  When
jprocess's filter sees a whole JSONRPC message, it calls the message
handler.

> Joao: "markers in the process buffer is what is used commonly"

Markers are positions in the buffer that maintain relative positions if
you add text before them.  Point is a marker, and so is the
process-mark, which marks where process last left input.  With that
concept in place, it's usually easi(er) to code processing of messages.

João