bug#61350: Eglot over Tramp freezes with large project

[João Távora]

Michael Albinus <michael.albinus@gmx.de> writes:

> Hi João,
>
>>> You don't call jsonrpc--notification-dispatcher anymore in the
>>> process filter directly. But calling it by a timer has the same effect:
>>> The process filter (accepting output) is still active,
>>
>> What exactly do you mean by "still active" and what process are you
>> referring to?  Jsonrpc's or Tramp?
>
> Your timer is called immediately, while you're still in
> jsonrpc--process-filter I believe.

I don't think so.  Any timer function runs in its stack, after the stack
where jsonrpc--process-filter happened has unwound.  But you've answered
my question: by "active" you mean "under the stack frame of
jsonrpc--process-filter".

>>> > Since jsonrpc always accepts output from *all* running processes, there
>>> > could be (and is!) the constellation, that process output has been read
>>> > already, and Tramp didn't get it, waiting for the output forever.
>>
>> I could understand if we were talking C and read() here, but the process
>> output of other processes read by jsonrpc's call to accept-process-output
>> surely must have run through some filter function, it wasn't just lost
>> AFAIK.  You've probably seen this trick a mililon times, but markers
>> in the process buffer is what is used commonly.
>
> It wasn't lost. The process output was retrieved and placed into the
> Tramp buffer, w/o Tramp's interaction.

That's great and quite normal.

> Tramp doesn't use a process filter for its own connections.

Every process in Emacs has a process filter, though it might not be a
custom process filter.  If you don't give it one, it is assigned
internal-default-process-filter, which as the docstring explains, simply
inserts process output into the buffer (of course you probably know
this, I'm just stating for other readers).

> is rather, that Tramp must know where the output to be parsed starts in
> the buffer.

Right, and this is where 'point' and 'process-mark' come in.  Process
mark is where the internal filter last left the insertion, and point is
where you the programmer last left your parsing.

> If another process has consumed the output, even if it is pushed into
> the correct buffer, Tramp doesn't know.

Why?  May I ask -- perhaps naively -- why can't you can't just

  (with-current-buffer proc (= (point) (process-mark)))

to "know"?

In my experience, something like this is usually sufficient.  One parses
the buffer for incoming messages looking for regexps, magic byte
sequences, etc.  One always leaves point after a successful search
(search-forward-regexp has this behaviour and it's very convenient).
Eventually, point may be left exactly at process-mark, or not, depending
on how much data came in, a full message, multiple full messages, or
some fractional message.

Regardless, next time you want to get messages from the process, you
perform a check before you go on a potentially blocking call to fetch
more output.  The check is usually "has process-mark advanced behind my
back from other libraries I don't control?"  Here, jsonrpc.el's a-p-o
calls are the "behing your back".  After checking, you know if you have
enough data to form a full message, or if you need to go on a
potentially blocking a-p-o call.

But somehow I suspect you know all this by heart already!!  In fact,
from the backtrace you posted Fri, 17 Feb 2023, it's clear the hang
happend in 'tramp-wait-for-regexp' whic starts 

(defun tramp-wait-for-regexp (proc timeout regexp)
  ...
  (let ((found (tramp-check-for-regexp proc regexp)))
    (cond (timeout
           (with-timeout (timeout)
             (while (not found)
               (tramp-accept-process-output proc)

Here, exactly how I imaged, you first check for "found" before venturing
into the blocking tramp-accept-process-output call.  So it looks to me
that you're doing conceptually the right thing, but it's
tramp-check-for-regexp who is missing the fact that there is a perfectly
good message in process buffer already.  At least according to what I
understood from your account of the problem.

So my suspicion is in tramp-check-for-regexp.  I found it a bit hard to
read to find an obvious culprit, and I still haven't a working setup to
reproduce this...

>>> Again, I still believe we need a general solution in Tramp, using threads.
>>
>> I don't understand what is conceptually impossible (or very hard) to
>> achieve with evented IO like we have in Emacs.
>
> I've tried different patches, mainly in tramp-accept-process-output. It
> improves the situation a little bit, but not reliably. Sometimes the
> test works, sometimes it blocks. And even if it doesn't block, a while
> later we run into the "Forbidden reentrant call of Tramp" error.

I had recently a problem with reentrant calls in
jsonrpc--process-filter.  This is why you find a run-with-timer call
there, right at the beginning.  This removes the reentrancy because as
written before, timers run in their own stack.  This was the fix to a
nasty Eglot bug#60088 with similar "hanging" behaviour.

> Honestly, I still don't understand really what's up. Let's see whether
> adding threads to Tramp helps.

I'll try to setup a VM myself with the reproduction recipe that Thomas
used.  I'm reasonably confident that two process-based extensions such
as Jsonrpc.el and TRAMP can coeexist if each follows process etiquette.

João