From ddb146fd0d34a371871ddf7e697f31507a23c3b6 Mon Sep 17 00:00:00 2001 From: Sam Herrmann Date: Tue, 19 Aug 2025 10:19:52 -0400 Subject: [PATCH 1/4] Cancel Handler context when connection closes (#90) --- conn.go | 18 ++++++++++-- conn_test.go | 81 ++++++++++++++++++++++++++++++++++++++++------------ jsonrpc2.go | 8 +++--- 3 files changed, 83 insertions(+), 24 deletions(-) diff --git a/conn.go b/conn.go index 18466cb..35c6279 100644 --- a/conn.go +++ b/conn.go @@ -27,6 +27,7 @@ type Conn struct { sending sync.Mutex + cancelCtx context.CancelFunc disconnect chan struct{} logger Logger @@ -43,13 +44,19 @@ var _ JSONRPC2 = (*Conn)(nil) // JSON-RPC protocol is symmetric, so a Conn runs on both ends of a // client-server connection. // -// NewClient consumes conn, so you should call Close on the returned -// client not on the given conn. +// NewConn consumes stream, so you should call Close on the returned +// Conn not on the given stream or its underlying connection. +// +// Conn is closed when the given context's Done channel is closed. func NewConn(ctx context.Context, stream ObjectStream, h Handler, opts ...ConnOpt) *Conn { + + ctx, cancel := context.WithCancel(ctx) + c := &Conn{ stream: stream, h: h, pending: map[ID]*call{}, + cancelCtx: cancel, disconnect: make(chan struct{}), logger: log.New(os.Stderr, "", log.LstdFlags), } @@ -60,6 +67,12 @@ func NewConn(ctx context.Context, stream ObjectStream, h Handler, opts ...ConnOp opt(c) } go c.readMessages(ctx) + + go func() { + <-ctx.Done() + c.close(nil) + }() + return c } @@ -182,6 +195,7 @@ func (c *Conn) close(cause error) error { } close(c.disconnect) + c.cancelCtx() c.closed = true return c.stream.Close() } diff --git a/conn_test.go b/conn_test.go index aa32fd9..5d2a7e4 100644 --- a/conn_test.go +++ b/conn_test.go @@ -14,6 +14,58 @@ import ( "github.com/sourcegraph/jsonrpc2" ) +func TestConn(t *testing.T) { + + t.Run("closes when context is done", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + + connA, connB := Pipe(ctx, noopHandler{}, noopHandler{}) + defer connA.Close() + defer connB.Close() + + cancel() + <-connA.DisconnectNotify() + + got := connA.Close() + want := jsonrpc2.ErrClosed + if got != want { + t.Fatalf("got %v, want %v", got, want) + } + }) + + t.Run("cancels context when closed", func(t *testing.T) { + ctxCanceled := make(chan struct{}) + + handler := handlerFunc(func(ctx context.Context, c *jsonrpc2.Conn, r *jsonrpc2.Request) { + // Block until the context is canceled. + <-ctx.Done() + close(ctxCanceled) + }) + + connA, connB := Pipe(context.Background(), noopHandler{}, jsonrpc2.AsyncHandler(handler)) + defer connA.Close() + defer connB.Close() + + // Send a notification from connA to connB to trigger connB's handler + // function. + if err := connA.Notify(context.Background(), "foo", nil, nil); err != nil { + t.Fatal(err) + } + + // Disconnect connA from connB. + if err := connA.Close(); err != nil { + t.Fatal(err) + } + + select { + case <-ctxCanceled: + // Test passed, the handler's context was canceled. + case <-time.After(time.Second): + t.Fatal("context not canceled") + } + }) +} + var paramsTests = []struct { sendParams interface{} wantParams *json.RawMessage @@ -198,12 +250,12 @@ func testParams(t *testing.T, want *json.RawMessage, fn func(c *jsonrpc2.Conn) e wg.Done() }) - client, server := newClientServer(handler) - defer client.Close() - defer server.Close() + connA, connB := Pipe(context.Background(), noopHandler{}, handler) + defer connA.Close() + defer connB.Close() wg.Add(1) - if err := fn(client); err != nil { + if err := fn(connA); err != nil { t.Error(err) } wg.Wait() @@ -242,18 +294,11 @@ func assertRawJSONMessage(t *testing.T, got *json.RawMessage, want *json.RawMess } } -func newClientServer(handler jsonrpc2.Handler) (client *jsonrpc2.Conn, server *jsonrpc2.Conn) { - ctx := context.Background() - connA, connB := net.Pipe() - client = jsonrpc2.NewConn( - ctx, - jsonrpc2.NewPlainObjectStream(connA), - noopHandler{}, - ) - server = jsonrpc2.NewConn( - ctx, - jsonrpc2.NewPlainObjectStream(connB), - handler, - ) - return client, server +// Pipe returns two jsonrpc2.Conn, connected via a synchronous, in-memory, full +// duplex network connection. +func Pipe(ctx context.Context, handlerA, handlerB jsonrpc2.Handler) (connA *jsonrpc2.Conn, connB *jsonrpc2.Conn) { + a, b := net.Pipe() + connA = jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(a), handlerA) + connB = jsonrpc2.NewConn(ctx, jsonrpc2.NewPlainObjectStream(b), handlerB) + return connA, connB } diff --git a/jsonrpc2.go b/jsonrpc2.go index 97e26d7..7d3e132 100644 --- a/jsonrpc2.go +++ b/jsonrpc2.go @@ -59,10 +59,10 @@ const ( // Handler handles JSON-RPC requests and notifications. type Handler interface { - // Handle is called to handle a request. No other requests are handled - // until it returns. If you do not require strict ordering behavior - // of received RPCs, it is suggested to wrap your handler in - // AsyncHandler. + // Handle is called to handle a request. No other requests are handled until + // it returns. If you do not require strict ordering behavior of received + // RPCs, it is suggested to wrap your handler in AsyncHandler. The context + // is automatically canceled when the connection closes. Handle(context.Context, *Conn, *Request) } From 3c4c92ad61e8a64c37816d2c573f5d0094d96d33 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 2 Sep 2025 13:36:38 +0200 Subject: [PATCH 2/4] Bump actions/checkout from 4 to 5 in the github-actions group (#91) Bumps the github-actions group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 4 to 5 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- .github/workflows/scip.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 633144a..e38deb3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: name: Go ${{ matrix.go }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up Go uses: actions/setup-go@v5 with: diff --git a/.github/workflows/scip.yml b/.github/workflows/scip.yml index 24a4a0e..4d72d7e 100644 --- a/.github/workflows/scip.yml +++ b/.github/workflows/scip.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-latest container: sourcegraph/scip-go steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Get src-cli run: curl -L https://sourcegraph.com/.api/src-cli/src_linux_amd64 -o /usr/local/bin/src; chmod +x /usr/local/bin/src From ef3ea8b2ea04744e4806bd2de8e9fdb2de1b2a73 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 2 Oct 2025 20:55:33 +0200 Subject: [PATCH 3/4] Bump actions/setup-go from 5 to 6 in the github-actions group (#92) Bumps the github-actions group with 1 update: [actions/setup-go](https://github.com/actions/setup-go). Updates `actions/setup-go` from 5 to 6 - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](https://github.com/actions/setup-go/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major dependency-group: github-actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e38deb3..66ba42e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,7 @@ jobs: steps: - uses: actions/checkout@v5 - name: Set up Go - uses: actions/setup-go@v5 + uses: actions/setup-go@v6 with: go-version: ${{ matrix.go }} id: go From 4756698b1e49a0d796ed81be119fd5bafdf7ff16 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 2 Dec 2025 09:01:38 +0200 Subject: [PATCH 4/4] Bump actions/checkout from 5 to 6 in the github-actions group (#93) --- .github/workflows/ci.yml | 2 +- .github/workflows/scip.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66ba42e..29413b6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: name: Go ${{ matrix.go }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Set up Go uses: actions/setup-go@v6 with: diff --git a/.github/workflows/scip.yml b/.github/workflows/scip.yml index 4d72d7e..c0dc33b 100644 --- a/.github/workflows/scip.yml +++ b/.github/workflows/scip.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-latest container: sourcegraph/scip-go steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - name: Get src-cli run: curl -L https://sourcegraph.com/.api/src-cli/src_linux_amd64 -o /usr/local/bin/src; chmod +x /usr/local/bin/src