Bump github.com/gorilla/websocket from 1.4.1 to 1.5.3 (#102)

Closing pending calls must also remove them from response routing; otherwise a response already read from the stream can later send into a closed waiter channel. Keep registration and shutdown mutually exclusive so the pending map retains that invariant.

.github/workflows/ci.yml

@@ -20,7 +20,7 @@ jobs:
     steps:
       - uses: actions/checkout@v7
       - name: Set up Go
-        uses: actions/setup-go@v6
+        uses: actions/setup-go@v7
         with:
           go-version: ${{ matrix.go }}
         id: go

.github/workflows/scip.yml

@@ -1,20 +0,0 @@
-name: SCIP
-'on':
-  - push
-permissions:
-  contents: read
-jobs:
-  scip-go:
-    runs-on: ubuntu-latest
-    container: sourcegraph/scip-go
-    steps:
-      - uses: actions/checkout@v7
-      - 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
-      - name: Set directory to safe for git
-        run: git config --global --add safe.directory $GITHUB_WORKSPACE
-      - name: Generate SCIP data
-        run: scip-go
-      - name: Upload SCIP data
-        run: src code-intel upload -github-token=${{ secrets.GITHUB_TOKEN }}

async.go

@@ -3,7 +3,8 @@ package jsonrpc2
 import "context"
 
 // AsyncHandler wraps a Handler such that each request is handled in its own
-// goroutine. It is a convenience wrapper.
+// goroutine. Handlers may therefore start or finish in a different order than
+// requests are received. It is a convenience wrapper.
 func AsyncHandler(h Handler) Handler {
 	return asyncHandler{h}
 }

conn.go

@@ -156,7 +156,9 @@ func (c *Conn) Notify(ctx context.Context, method string, params interface{}, op
 	return err
 }
 
-// Reply sends a successful response with a result.
+// Reply sends a successful response with a result. It returns after writing
+// the response to the ObjectStream; it does not wait for the peer to receive or
+// process the response.
 func (c *Conn) Reply(ctx context.Context, id ID, result interface{}) error {
 	resp := &Response{ID: id}
 	if err := resp.SetResult(result); err != nil {
@@ -186,8 +188,9 @@ func (c *Conn) close(cause error) error {
 	}
 	c.closed = true
 
-	for _, call := range c.pending {
+	for id, call := range c.pending {
 		close(call.done)
+		delete(c.pending, id)
 	}
 
 	close(c.disconnect)
@@ -318,6 +321,10 @@ func (c *Conn) send(_ context.Context, m *anyMessage, wait bool) (cc *call, err
 	// responses.
 	if m.request != nil && wait {
 		c.mu.Lock()
+		if c.closed {
+			c.mu.Unlock()
+			return nil, ErrClosed
+		}
 		id = m.request.ID
 		c.pending[id] = cc
 		c.mu.Unlock()

conn_test.go

@@ -275,6 +275,78 @@ func TestConn_Close(t *testing.T) {
 	}
 }
 
+func TestConn_CloseAfterResponseRead(t *testing.T) {
+	client, server := net.Pipe()
+	responseRead := make(chan struct{})
+	processResponse := make(chan struct{})
+	responseIgnored := make(chan struct{})
+	var resumeResponse sync.Once
+	resume := func() { resumeResponse.Do(func() { close(processResponse) }) }
+	defer resume()
+
+	clientStream := &pauseAfterReadStream{
+		ObjectStream: jsonrpc2.NewPlainObjectStream(client),
+		read:         responseRead,
+		resume:       processResponse,
+	}
+	clientConn := jsonrpc2.NewConn(
+		context.Background(),
+		clientStream,
+		noopHandler{},
+		jsonrpc2.SetLogger(signalLogger{called: responseIgnored}),
+	)
+	serverConn := jsonrpc2.NewConn(
+		context.Background(),
+		jsonrpc2.NewPlainObjectStream(server),
+		handlerFunc(func(ctx context.Context, conn *jsonrpc2.Conn, req *jsonrpc2.Request) {
+			if err := conn.Reply(ctx, req.ID, nil); err != nil {
+				t.Error(err)
+			}
+		}),
+	)
+	defer serverConn.Close()
+
+	call, err := clientConn.DispatchCall(context.Background(), "m", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	<-responseRead
+
+	if err := clientConn.Close(); err != nil {
+		t.Fatal(err)
+	}
+	resume()
+	<-responseIgnored
+
+	if err := call.Wait(context.Background(), nil); err != jsonrpc2.ErrClosed {
+		t.Fatalf("got error %v, want %v", err, jsonrpc2.ErrClosed)
+	}
+}
+
+type pauseAfterReadStream struct {
+	jsonrpc2.ObjectStream
+	read   chan<- struct{}
+	resume <-chan struct{}
+	once   sync.Once
+}
+
+func (s *pauseAfterReadStream) ReadObject(v interface{}) error {
+	err := s.ObjectStream.ReadObject(v)
+	s.once.Do(func() {
+		close(s.read)
+		<-s.resume
+	})
+	return err
+}
+
+type signalLogger struct {
+	called chan<- struct{}
+}
+
+func (l signalLogger) Printf(string, ...interface{}) {
+	close(l.called)
+}
+
 func testParams(t *testing.T, want *json.RawMessage, fn func(c *jsonrpc2.Conn) error) {
 	wg := &sync.WaitGroup{}
 	handler := handlerFunc(func(ctx context.Context, conn *jsonrpc2.Conn, r *jsonrpc2.Request) {

go.mod

@@ -2,4 +2,4 @@ module github.com/sourcegraph/jsonrpc2
 
 go 1.12
 
-require github.com/gorilla/websocket v1.4.1
+require github.com/gorilla/websocket v1.5.3

go.sum

@@ -1,2 +1,2 @@
-github.com/gorilla/websocket v1.4.1 h1:q7AeDBpnBk8AogcD4DSag/Ukw/KV+YhzLj2bP5HvKCM=
-github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
+github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
+github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=

jsonrpc2.go

@@ -59,10 +59,12 @@ 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. The context
-	// is automatically canceled when the connection closes.
+	// Handle is called to handle a request. While it is running, the connection
+	// does not process any other requests or responses. A handler that waits for
+	// a call's response must therefore run asynchronously. 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)
 }