Skip to content

Commit

Permalink
Fix server connection leak after receive message error
Browse files Browse the repository at this point in the history
When a receive error occurs, the goroutine and channel for receiving
messages server-side is stopped; however, before the server would only
clean up on a subset of these errors.

Signed-off-by: Austin Vazquez <[email protected]>
  • Loading branch information
austinvazquez committed May 1, 2023
1 parent 98b5f64 commit 40e8044
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 12 deletions.
4 changes: 2 additions & 2 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,15 +323,15 @@ func (c *Client) receiveLoop() error {
sid := streamID(msg.header.StreamID)
s := c.getStream(sid)
if s == nil {
logrus.WithField("stream", sid).Errorf("ttrpc: received message on inactive stream")
logrus.WithField("stream", sid).Error("ttrpc: received message on inactive stream")
continue
}

if err != nil {
s.closeWithError(err)
} else {
if err := s.receive(c.ctx, msg); err != nil {
logrus.WithError(err).WithField("stream", sid).Errorf("ttrpc: failed to handle message")
logrus.WithError(err).WithField("stream", sid).Error("ttrpc: failed to handle message")
}
}
}
Expand Down
15 changes: 5 additions & 10 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,10 @@ package ttrpc

import (
"context"
"errors"
"io"
"math/rand"
"net"
"sync"
"sync/atomic"
"syscall"
"time"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -546,15 +543,13 @@ func (c *serverConn) run(sctx context.Context) {
// TODO(stevvooe): Not wildly clear what we should do in this
// branch. Basically, it means that we are no longer receiving
// requests due to a terminal error.
recvErr = nil // connection is now "closing"
if err == io.EOF || err == io.ErrUnexpectedEOF || errors.Is(err, syscall.ECONNRESET) {
// The client went away and we should stop processing
// requests, so that the client connection is closed
return
}

// The client went away and we should stop processing
// requests, so that the client connection is closed
logrus.WithError(err).Error("error receiving message")
// else, initiate shutdown
return
case <-shutdown:
// initiate shutdown
return
}
}
Expand Down

0 comments on commit 40e8044

Please sign in to comment.