GNU bug report logs - #67945
30.0.50; Jsonrpc trouble dealing with nested synchronous requests

Previous Next

To reply to this bug, email your comments to 67945 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: "simon254--- via Bug reports for GNU Emacs,
 the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>,
 Daniel Pettersson <daniel <at> dpettersson.net>
Subject: 30.0.50; Jsonrpc trouble dealing with nested synchronous requests
Date: Wed, 20 Dec 2023 23:30:08 +0000

[Message part 1 (text/plain, inline)]

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.

[jbug.py (application/octet-stream, attachment)]

[jbug.el (application/octet-stream, attachment)]

Message #8 received at 67945 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: 67945 <at> debbugs.gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#67945: 30.0.50; Jsonrpc trouble dealing with nested
 synchronous requests
Date: Thu, 21 Dec 2023 01:01:44 +0000

On Wed, Dec 20, 2023 at 11:31 PM João Távora <joaotavora <at> gmail.com> wrote:
>
> 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've now pushed the fix to jsonrpc.el and tagged version 1.0.21
of the jsonrpc package.

I'd like to push some tests using the `jbug.py` test server
attached earlier.  Is here a good place to store such helper scripts
in our test subtree?

João

Message #11 received at 67945 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: 67945 <at> debbugs.gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#67945: 30.0.50;
 Jsonrpc trouble dealing with nested synchronous requests
Date: Thu, 21 Dec 2023 09:51:20 +0200

> From: João Távora <joaotavora <at> gmail.com>
> Date: Thu, 21 Dec 2023 01:01:44 +0000
> 
> I'd like to push some tests using the `jbug.py` test server
> attached earlier.  Is here a good place to store such helper scripts
> in our test subtree?

In the FOO-resources subdirectory (where FOO is the name of the test)?

Message #14 received at 67945 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67945 <at> debbugs.gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#67945: 30.0.50; Jsonrpc trouble dealing with nested
 synchronous requests
Date: Thu, 21 Dec 2023 09:52:51 +0000

On Thu, Dec 21, 2023 at 7:51 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: João Távora <joaotavora <at> gmail.com>
> > Date: Thu, 21 Dec 2023 01:01:44 +0000
> >
> > I'd like to push some tests using the `jbug.py` test server
> > attached earlier.  Is here a good place to store such helper scripts
> > in our test subtree?
>
> In the FOO-resources subdirectory (where FOO is the name of the test)?

Yes, that was my idea too.  Thanks for confirming.

Message #17 received at 67945 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 67945 <at> debbugs.gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#67945: 30.0.50; Jsonrpc trouble dealing with nested
 synchronous requests
Date: Thu, 1 Feb 2024 23:40:59 -0800

João Távora <joaotavora <at> gmail.com> writes:

> On Wed, Dec 20, 2023 at 11:31 PM João Távora <joaotavora <at> gmail.com> wrote:
>>
>> 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've now pushed the fix to jsonrpc.el and tagged version 1.0.21
> of the jsonrpc package.
>
> I'd like to push some tests using the `jbug.py` test server
> attached earlier.  Is here a good place to store such helper scripts
> in our test subtree?
>
> João

Thanks, should this bug be closed or is there more to do here?

Message #20 received at 67945 <at> debbugs.gnu.org (full text, mbox):

From: João Távora <joaotavora <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 67945 <at> debbugs.gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#67945: 30.0.50; Jsonrpc trouble dealing with nested
 synchronous requests
Date: Fri, 2 Feb 2024 09:29:28 +0000

On Fri, Feb 2, 2024 at 7:41 AM Stefan Kangas <stefankangas <at> gmail.com> wrote:
>
> João Távora <joaotavora <at> gmail.com> writes:
>
> > On Wed, Dec 20, 2023 at 11:31 PM João Távora <joaotavora <at> gmail.com> wrote:
> >>
> >> 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've now pushed the fix to jsonrpc.el and tagged version 1.0.21
> > of the jsonrpc package.
> >
> > I'd like to push some tests using the `jbug.py` test server
> > attached earlier.  Is here a good place to store such helper scripts
> > in our test subtree?
> >
> > João
>
> Thanks, should this bug be closed or is there more to do here?

This is fixed for the somewhat limited manual testing I did.

But!

* the automated tests are not in place.  They shouldn't be hard to
  add though, and Eli's suggestion to put jbug.py somewhere in a
  "resources" directory is perfectly acceptable.

* Daniel Pettersson, who is now using jsonrpc.el in his DAP extension,
  tried to do more even more advanced stuff with this jsonrpc.el
  version and hit an internal sanity assertion, i.e. most assuredly
  a bug.  I haven't yet reproduced it.

* I think Daniel Pettersson would make a fine jsonrpc.el maintainer :-)
  <triple wink>

João

Message #23 received at 67945 <at> debbugs.gnu.org (full text, mbox):

From: Daniel Pettersson <daniel <at> dpettersson.net>
To: João Távora <joaotavora <at> gmail.com>
Cc: 67945 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#67945: 30.0.50; Jsonrpc trouble dealing with nested
 synchronous requests
Date: Sun, 4 Feb 2024 13:32:46 +0100

> * I think Daniel Pettersson would make a fine jsonrpc.el maintainer :-)
>  <triple wink>

I might be able to take it on, I am not familiar with the mechanics of
maintaining an core package but I might be up for it.

Daniel

Message #26 received at 67945 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 67945 <at> debbugs.gnu.org, daniel <at> dpettersson.net
Subject: Re: bug#67945: 30.0.50; Jsonrpc trouble dealing with nested
 synchronous requests
Date: Thu, 25 Jul 2024 00:12:09 -0700

João Távora <joaotavora <at> gmail.com> writes:

> This is fixed for the somewhat limited manual testing I did.
>
> But!
>
> * the automated tests are not in place.  They shouldn't be hard to
>   add though, and Eli's suggestion to put jbug.py somewhere in a
>   "resources" directory is perfectly acceptable.
>
> * Daniel Pettersson, who is now using jsonrpc.el in his DAP extension,
>   tried to do more even more advanced stuff with this jsonrpc.el
>   version and hit an internal sanity assertion, i.e. most assuredly
>   a bug.  I haven't yet reproduced it.

Daniel, is this something you could take a look at?

Thanks in advance.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.