Subject: Re: bug#70522: 29.2.50; eglot-shutdown sends SIGKILL before LSP server can exit gracefully

From: Daniel Pettersson <[email-masked]>
Date: Tue, 23 Apr 2024 18:58:06 +0200

Eli Zaretskii <[email-masked]> writes:

> Daniel, could you please look into this issue?

Change makes sense, the only "risk" is that jsonrpc users populate the
*Warnings* buffer with an warning that they should take action upon and
solve, so I think this is an no brainer.

Nice job!

I have done some adhock testing of eglot and dape. Was not able to
produce the warning, which means that eglot and dape are using the
function as intended (at least when interacting with the servers I
tested with).

When the function is touched I would like to do some small additions to
the documentation of the function, would be nice if the docstring
mention the undocumented behavior and an small comment to make it a bit
easier to follow along. 

diff --git a/lisp/jsonrpc.el b/lisp/jsonrpc.el
index 5037d8c5b2b..111e58cefe2 100644
--- a/lisp/jsonrpc.el
+++ b/lisp/jsonrpc.el
@@ -591,15 +591,18 @@ jsonrpc-running-p
 (cl-defmethod jsonrpc-shutdown ((conn jsonrpc-process-connection)
                                 &optional cleanup)
   "Wait for JSONRPC connection CONN to shutdown.
-With optional CLEANUP, kill any associated buffers."
+With optional CLEANUP, kill any associated buffers.
+If CONN is not shutdown within an reasonable amount of time, warn
+and delete process."
   (unwind-protect
       (cl-loop
        with proc = (jsonrpc--process conn) for i from 0
        while (not (process-get proc 'jsonrpc-sentinel-cleanup-started))
        unless (zerop i) do
        (jsonrpc--warn "Sentinel for %s still hasn't run, deleting it!" proc)
-       do
        (delete-process proc)
+       do
+       ;; Let sentinel have a chance to run
        (accept-process-output nil 0.1))
     (when cleanup
       (kill-buffer (process-buffer (jsonrpc--process conn)))

/ Daniel