Subject: bug#67945: 30.0.50; Jsonrpc trouble dealing with nested synchronous requests

From: João Távora <[email-masked]>
Date: Wed, 20 Dec 2023 23:30:08 +0000

Hi,

I've come across a rather complicated bug in jsonrpc.el that affects a
certain class of JSON-RPC applications.  Eglot, on of the more important
`jsonrpc.el` clients (perhaps the most) is not affected, as far as I
can tell.

I already have a fix for this bug, but I am filing this report as a
reference for the commit message and to collect comments.

I was made aware of this bug by Daniel Pettersson, who is experimenting
with integrating JSONRPC in his DAP client, dape.el.  DAP's based
transport protocol isn't really JSON-RPC based but jsonrpc.el can be
accomodated to support it.

However the bug can be triggered with a sufficiently insidious and plain
JSON-RPC application.  The attached files jbug.el and jbug.py worked on
by Daniel and me demonstrate the problem.

* jbug.py is a JSON-RPC server that upon receiving the ":ping"" request
  immediately notifies the client with a ":hello"" notification.  Then
  it waits briefly 0.1 seconds before reponding to the ":ping" request
  with ":pong".

* jbug.el is a JSON-RPC client that sends a ":ping" request and waits
  for a response.  While waiting for that response, it receives the
  ":hello" notification and issues a synchronous second ":slow" request
  inside the notification handler.  Both requests are synchronous and
  performed with the Elisp function 'jsonrpc-request'

As the name implies, the server takes longer to answer the ":slow"
request (about 0.5 seconds)

Before the patch that I'm about to push, the response to the ":slow"
request is never seen by the client.  The notification handler, which
runs in a timer, simply exists non-locally.

The reasons for this are somewhat complicated, but now well understood.

Briefly, the synchronicity of 'jsonrpc-request' is achieved by having
'accept-process-output' wrapped in a try-catch block.  A continuation
handler running in a timer will eventually throw to the catch tag, so
the next form after 'jsonrpc-request' can be executed.  During the wait,
JSON-RPC notifications may come in and they often do, but in most
JSON-RPC applications (Eglot, at least), the notification handlers don't
issue what will become a nested 'jsonrpc-request'.  And even if they
were to do that, they would only run into this problem if that second
inner request ends at a later time than the first request.

In dape.el and in the jbug.el file attached, this can happen.  The
result will be that the outer catch tag of the first outer request is
"thrown to" when the response arrives, but that also unwinds the stack
of the second inner request, which leads to the application never seeing
the response that will appear only later.

In the fix I'm about to push, the solution is to record the nesting in
an Elisp structure and prevent throwing to the outer request when its
response arrives.  Rather, we record that desired continuation to run
only when the inner request has resolved fully.  The throwing will
happen only when we are back at the first outer requests's
'accept-process-output'.

This makes both responses be seen by the JSON-RPC application, albeit in
reverse order.  I think this isn't a problem in real life, because from
the jsonrpc.el programmer's perspective a blocking jsonrpc-request call
is only meant to preserve the order of execution of the forms in which
that call.  It's _not_ the relative order of multiple "concurrent"
'jsonrpc-request' calls, because this concurrency isn't really defined
in the single-threaded Elisp programming model.

I attach the two files jbug.el and jbug.py.