tree 0379b83c1608c30d04b3fcec36b30fbd872f9b32
parent 0fd2d649e656450a551dd49c43b57425dffd4b53
author Rob Findley <rfindley@google.com> 1581442687 -0500
committer Robert Findley <rfindley@google.com> 1581691404 +0000

internal/jsonrpc2: fix races in cancellation

We had a deadlock in cases where a request was cancelled (1) after being
written to the stream, but (2) before a response was received. This
resulted in the request ID being removed from the pending map while the
server has the request, after which point the server response would hang
in Conn.Run trying to send to a nil channel.

After fixing this nil send there was still a race: it was possible that
Conn.Run could get the pending request, and Conn.Call would select
ctx.Done before Conn.Run could send to the response channel, again
resulting in a blocking send. Fix this by adding a buffer to the
response channel.

The response channel management is also made less forgiving, because we
should be able to reason precisely about how many sends and receives
will occur:
 + Don't close the response channel after sending a response: there
   should only be one recipient.
 + Don't delete the ID from pending map twice: it should only be cleaned
   up by Conn.Call.

Cancellation tests in the lsprpc package are updated to exercise the
race conditions.

Fixes golang/go#37159

Change-Id: Ie3207442ea910f79247b18d8647fd52f39fb15db
Reviewed-on: https://go-review.googlesource.com/c/tools/+/219126
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>