Process stdio mishandled in lsp-mode's stdio client

[resttime]

In the process of trying to add LSP support for c-mode through clangd a new issue appears. This is found to affect both SDL2 and ncurses frontends so it seems specific to backend.

First clangd only supports stdio but that's okay since there's an stdio client implemented for lsp-mode and the spec can be defined like this(?)

(define-language-spec (c-spec lem-c-mode:c-mode)
  :language-id "c"
  :root-uri-patterns '("Makefile")
  :command (lambda () '("clangd"))
  :readme-url "https://clangd.llvm.org/"
  :connection-mode :stdio)

However lsp-mode is stuck "initializing..." along with a modeline spinner spinning forever. Looking at the configuration for the go-mode language server gopls seems to work through TCP although so I tried to see if it'd be the same for clangd. I used socat to fork the clangd process and redirect stdio to a TCP port. lsp-mode can connect to a TCP server now that redirects to clangd process. The hack worked surprisingly and lsp-mode started working in the buffer.

(define-language-spec (c-spec lem-c-mode:c-mode)
  :language-id "c"
  :root-uri-patterns '("Makefile")
  :command (lambda (port) `("socat" ,(format nil "TCP-LISTEN:~a,fork" port) "exec:clangd"))
  :readme-url "https://clangd.llvm.org/"
  :connection-mode :tcp)

There must be a bug in how the stdio of processes are being handled, the JSONRPC, or etc. This continuation that gets passed does not get complete evaluation when the stdio client is used. If it did the spinner would stop spinning and initialization would finish:

lem/extensions/lsp-mode/lsp-mode.lisp

Lines 393 to 403 in d7ece6a

	(lambda (workspace)
	(add-workspace workspace)
	(set-trigger-characters workspace)
	(add-buffer-hooks buffer)
	(when continuation (funcall continuation))
	(let ((mode (ensure-mode-object
	(spec-mode
	(workspace-spec workspace)))))
	(initialized-workspace mode workspace))
	(spinner:stop-loading-spinner spinner)
	(redraw-display)))))))

stdio client

tcp client

[Sasanidas]

Interesting! I did add a lsp config for elixir that uses stdio and it worked fined:

(define-language-spec (elixir-spec lem-elixir-mode:elixir-mode)
  :language-id "elixir"
  :root-uri-patterns '("mix.exs")
  :command '("sh" "language_server.sh")
  :install-command ""
  :readme-url "https://github.com/elixir-lsp/elixir-ls"
  :connection-mode :stdio)

I'll like to finish the #974 issue, so we can gather more information about the process behind (and potencially get some output of the incoming calls)

[resttime]

Found a root cause which prob explains why the stdio client for elixir works. If clangd detects it's run in a "terminal" it outputs a message that lem tries parse as a header. Which throws an error because it does not follow the LSP protocol specification. The default behaviour on an error is to return nil.

lem/extensions/lsp-mode/lem-stdio-transport.lisp

Lines 47 to 50 in 9ee5cef

	(let* ((headers (handler-case (read-headers stream)
	(error ()
	;; プロセスを終了したときにread-headersでエラーが出るのでここでハンドリングする
	(return-from receive-message-using-transport nil))))

This causes the thread which is running the loop processing the output from clangd to terminate. Thus nothing read gets returned an initialization cannot proceed.

lem/extensions/lsp-mode/lem-stdio-transport.lisp

Lines 27 to 31 in 9ee5cef

	(bt:make-thread
	(lambda ()
	(run-reading-loop transport connection))
	:name "lem-lsp-mode/lem-stdio-transport reading")))
	connection))

Ultimately lem is opening the clangd process but not redirecting the process stdio as intended by clangd.

https://github.com/llvm/llvm-project/blob/c8387a31a4adfa9c29a578cf67321f756d3b4ac1/clang-tools-extra/clangd/tool/ClangdMain.cpp#L836

There are quick hacks that can been added to ignore/trim/throw away the message as a a solution, although it'd be specific to clangd only. The "correct" solution would be to understand how clangd checks it runs in terminal and modify how the process is being opened since that'd affect other processes similarly.

[seanfarley]

Does clangd think it's in a terminal for the same reason as outlined here?

[resttime]

That may be the case as PTY is mentioned a lot in the library lem uses.

https://github.com/lem-project/async-process/blob/9690530fc92b59636d9f17d821afa7697e7c8ca4/src/async-process.c#L38

[seanfarley]

Much like that article mentions, we should be defaulting to PTY-less so as to avoid these kind of difficult-to-debug issues!

[joaotavora]

The "correct" solution would be to understand how clangd checks it runs in terminal and modify how the process is being opened since that'd affect other processes similarly.

Just randomly popped here when checking out LEM. Are you sure clangd isn't really writing that message to stderr? Mine is.
I think there's nothing in the spec against that. clangd writes lots of stuff to stderr, which one can use to make a (very useful, IMHO) events log.

So the "correct" solution would be for JSONRPC over stdin/stdout, and keep stderr handy.

[stderr]  I[06:16:06.960] <-- textDocument/didOpen
[stderr]  I[06:16:06.976] System includes extractor: successfully executed /usr/bin/clang
[stderr]  	got includes: "/usr/lib/clang/16/include, /usr/local/include, /usr/include"
[stderr]  	got target: "x86_64-pc-linux-gnu"
[stderr]  I[06:16:06.977] --> textDocument/publishDiagnostics
[jsonrpc] e[06:16:06.996] <-- textDocument/publishDiagnostics 

(I'm the author of the Eglot LSP client for Emacs)

[resttime]

Ah right you are stderr is used. llvm::errs() is the stream for stderr the message is being sent through.

…

On Thu, Jan 4, 2024 at 06:21 João Távora ***@***.***> wrote: The "correct" solution would be to understand how clangd checks it runs in terminal and modify how the process is being opened since that'd affect other processes similarly. Just randomly popped here when checking out LEM. Are you sure clangd isn't really writing that message to stderr? Mine is. I think there's nothing in the spec against that. clangd writes lots of stuff to stderr, which one can use to make a (very useful, IMHO) events log. So the "correct" solution would be for JSONRPC over stdin/stdout, and keep stderr handy. [stderr] I[06:16:06.960] <-- textDocument/didOpen [stderr] I[06:16:06.976] System includes extractor: successfully executed /usr/bin/clang [stderr] got includes: "/usr/lib/clang/16/include, /usr/local/include, /usr/include" [stderr] got target: "x86_64-pc-linux-gnu" [stderr] I[06:16:06.977] --> textDocument/publishDiagnostics [jsonrpc] e[06:16:06.996] <-- textDocument/publishDiagnostics (I'm the author of the Eglot LSP client for Emacs) — Reply to this email directly, view it on GitHub <#1076 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAT2P6EV4HZISS6LX4HH4XLYM2NDTAVCNFSM6AAAAAA4RHHEEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZXGAYDSMZWGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[garlic0x1]

Here is a solution, this gets rid of stderr:

(define-language-spec (c-spec lem-c-mode:c-mode)
  :language-id "c"
  :root-uri-patterns '("compile-commands.json")
  :command `("bash" "-c" "clangd 2> /dev/null")
  :install-command ""
  :readme-url ""
  :connection-mode :stdio)