bug#61350: Eglot over Tramp freezes with large project

[João Távora]

On Sun, Mar 5, 2023 at 11:21 AM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> Hi João,
>
> >> Yes. But Tramp calls accept-process-output only, if it has send a
> >> command to the remote shell, and it expects something to be returned. At
> >> least the shell prompt.
> >
> > Yes.  Tramp is doing the right thing.  It really expects a response to
> > come.  And more often than not, it does.  But sometimes it doesn't, and
> > that's when we hang.
>
> After digging further, I believe I understand now why it hangs. We have
> the following scenario:
>
> - Both Eglot and Tramp use the same ssh connection with enabled ControlMaster.
>
> - Eglot gets JSON output from the remote LSP server. In
>   jsonrpc--process-filter, this output is handled. It includes a call to
>   file-truename, which triggers Tramp to send a request in its own
>   process to the remote side.
>
> - The remote side returns the answer for Tramp (shell output). However,
>   the ssh socket is still full of the jsonrpc process output, which
>   waits to be handled.
>
> - So the socket is blocked. The jsonrpc output cannot be read, because
>   jsonrpc--process-filter waits for the result of file-truename. And the
>   Tramp process output cannot be handled, because it is stuck in the
>   socket after the jsonrpc output.

OK, I think I understand. But here, the main point that makes this
deadlock possible is that tramp when called by the jsonrpc process
filter is calling accept-process-output with JUST-THIS-ONE set to true.
_That_ is why jsonrpc--process-filter cannot see the remaining output
that is stuckl in the ssh socket, as you describe.

> The proper solution is indeed to have two connections, and to refuse use
> of ControlMaster.

Here, I disagree.  I think, like Thomas, that the proper solution is to
stop passing JUST-THIS-ONE to tramp's a-p-output calls.

> So would you mind to add a hook to Eglot, which runs before calling
> make-process, but in the proper process buffer? When you've added it,
> I'll let Tramp hook into, and you don't need any longer the Tramp
> specific code in eglot.el.

I don't think using this hook for this is the right idea, and I'm
fine with having that bit of Tramp-specific code in there until
we find a better solution.  So no need to worry about that.

João