1
0
mirror of https://github.com/golang/go synced 2024-11-05 18:26:10 -07:00
Commit Graph

5 Commits

Author SHA1 Message Date
Ian Cottrell
6dc6d5718f internal/jsonrpc2: change handler to a function type
Handler is now a function type that mapps to what used to be the Deliver method.
The only handler that used other methods was Canceller, for now that still
exists as LegacyHooks. Once the handlers are fully cleaned up we should be able
to re-implement canceller as handler middleware.
Each connection is now only allowed one handler, and it is passed to the Run
method, but handlers are composable.

Change-Id: I370e0459df851bb9c9c2a679b99cff073b94489e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226479
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-06 13:48:45 +00:00
Rob Findley
9ffc0ab4ef internal/jsonrpc2: add an idle timeout for stream serving
When running gopls against an automatically started remote instance, we
want the lifecycle of the remote to be detached from that of its
clients, so that it doesn't shut down while clients are still connected.
On the other hand, a gopls process can consume significant resources, so
we don't want it to remain when there are no more connected clients.

The jsonrpc2 package is updated to support the concept of idle timeout:
a duration after which the server is shut down when there are no
connected clients. This is exposed in the gopls serve command via the
-listen.timeout flag.

Update golang/go#34111

Change-Id: Id62b3d4a2fa66de2c9306d130ca431717f01d1e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220281
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-24 23:06:35 +00:00
Rob Findley
66de428735 internal/jsonrpc2,internal/lsp/regtest: clean up some leaked tempfiles
Not all regtests resulted in LSP shutdown, which caused temp modfiles to
be leaked. After this fix I have confirmed that /tmp is clean after a
successful run of the regtests.

Also proactively clean up the unix socket file when serving jsonrpc2
over UDS.

Change-Id: I745fbd3d2adeeb165cadf7c54fd815d8df81d4e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/220061
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rohan Challa <rohan@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-19 18:32:50 +00:00
Rob Findley
5fb17a1e7b internal/jsonrpc2: support serving over unix domain sockets
For tests (and perhaps later, for daemon discovery), unix domain sockets
offer advantages over TCP: we can know the exact socket address that will be
used when starting a server subprocess. They also offer performance and
security advantages over TCP, and were specifically requested on
golang.org/issues/34111.

This CL adds support for listening on UDS, and uses this to implement an
additional regtest environment mode that starts up an external process.
This mode is disabled by default, but may be enabled by the
-enable_gopls_subprocess_tests.

The regtest TestMain may be hijacked to instead run as gopls, if a
special environment variable is set. This allows the the test runner to
start a separate process by using os.Argv[0]. The -gopls_test_binary
flag may be used to point tests at a separate gopls binary.

Updates golang/go#36879
Updates golang/go#34111

Change-Id: I1cfdf55040e81ffa69a6726878a96529e5522e82
Reviewed-on: https://go-review.googlesource.com/c/tools/+/218839
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-19 16:14:01 +00:00
Rob Findley
c29062fe1d internal/lsp: refactor LSP server instantiation
Previously, the process of instantiating and running the LSP server was
sharded across the lsp, protocol, and cmd packages, and this resulted in
some APIs that are hard to work with. For example, it's hard to guess
the difference between lsp.NewClientServer, lsp.NewServer,
protocol.NewServer (which returns a client), and protocol.NewClient
(which returns a server).

This change reorganizes Server instantiation as follows:

 + The lsp.Server is now purely an implementation of the protocol.Server
   interface. It is no longer responsible for installing itself into the
   jsonrpc2 Stream, nor for running itself.

 + A new package 'lsprpc' is added, to implement the logic of binding an
   incoming connection to an LSP server session. This is put in a
   separate package for lack of a clear home: it didn't really
   philosophically belong in any of the lsp, cmd, or protocol packages.
   We can perhaps move it to cmd in the future, but I'd like to keep it
   as a separate package while I develop request forwarding.

   simplified import graph:

    jsonrpc2 ⭠ lsprpc ⭠ cmd
               ⭩           ⭦
            lsp           (t.b.d. client tests)
           ⭩   ⭨
     protocol  source

 + The jsonrpc2 package is extended to have a minimal API for running a
   'StreamServer': something analogous to an HTTP server that listens
   for new connections and delegates to a handler (but we couldn't use
   the word 'Handler' for this delegate as it was already taken).

After these changes, I hope that the concerns of "serving the LSP",
"serving jsonrpc2", and "installing the LSP on jsonrpc2" are more
logically organized, though one legitimate criticism is that the word
'Server' is still heavily overloaded.

This change prepares a subsequent change which hijacks the jsonrpc2
connection when forwarding messages to a shared gopls instance.

To test this change, the following improvements are made:

 + A servertest package is added to make it easier to run a test against
   an in-process jsonrpc2 server. For now, this uses TCP but it could
   easily be modified to use io.Pipe.

 + cmd tests are updated to use the servertest package. Unfortunately it
   wasn't yet possible to eliminate the concept of `remote=internal` in
   favor of just using multiple sessions, because view initialization
   involves calling both `go env` and `packages.Load`, which slow down
   session startup significantly. See also golang.org/issue/35968.

   Instead, the syntax for `-remote=internal` is modified to be
   `-remote=internal@127.0.0.1:12345`.

 + An additional test for request cancellation is added for the
   sessionserver package. This test uncovered a bug: when calling
   Canceller.Cancel, we were using id rather than &id, which resulted in
   incorrect json serialization (as only the pointer receiver implements
   the json.Marshaller interface).

Updates golang/go#34111

Change-Id: I75c219df634348cdf53a9e57839b98588311a9ef
Reviewed-on: https://go-review.googlesource.com/c/tools/+/215742
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-02-06 23:12:37 +00:00