Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle EOF to prevent file descriptor leak #37

Merged
merged 1 commit into from
May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,11 @@ func (c *serverConn) run(sctx context.Context) {
if err != nil && err != io.EOF {
logrus.WithError(err).Error("error receiving message")
}
if err == io.EOF || err == io.ErrUnexpectedEOF {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The err == io.ErrUnexpectedEO won't work unless you ignore it above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would not have worked if there was a return statement in the if above, but there isn't. It would still be better to not log the ErrUnecpectedEOF though so we have created a new pr: #38. Pls, take a look.

// The client went away and we should stop processing
// requests, so that the client connection is closed
return
}
case <-shutdown:
return
}
Expand Down
62 changes: 62 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"github.com/gogo/protobuf/proto"
"github.com/pkg/errors"
"github.com/prometheus/procfs"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -359,6 +360,48 @@ func TestClientEOF(t *testing.T) {
}
}

func TestServerEOF(t *testing.T) {
var (
ctx = context.Background()
server = mustServer(t)(NewServer())
addr, listener = newTestListener(t)
client, cleanup = newTestClient(t, addr)
)
defer cleanup()
defer listener.Close()

socketCountBefore := socketCount(t)

go server.Serve(ctx, listener)

registerTestingService(server, &testingServer{})

tp := &testPayload{}
// do a regular call
if err := client.Call(ctx, serviceName, "Test", tp, tp); err != nil {
t.Fatalf("unexpected error during test call: %v", err)
}

// close the client, so that server gets EOF
if err := client.Close(); err != nil {
t.Fatalf("unexpected error while closing client: %v", err)
}

// server should eventually close the client connection
maxAttempts := 20
for i := 1; i <= maxAttempts; i++ {
socketCountAfter := socketCount(t)
if socketCountAfter < socketCountBefore {
break
}
if i == maxAttempts {
t.Fatalf("expected number of open sockets to be less than %d after client close, got %d open sockets",
socketCountBefore, socketCountAfter)
}
time.Sleep(100 * time.Millisecond)
}
}

func TestUnixSocketHandshake(t *testing.T) {
var (
ctx = context.Background()
Expand Down Expand Up @@ -541,3 +584,22 @@ func mustServer(t testing.TB) func(server *Server, err error) *Server {
return server
}
}

func socketCount(t *testing.T) int {
proc, err := procfs.Self()
if err != nil {
t.Fatalf("unexpected error while reading procfs: %v", err)
}
fds, err := proc.FileDescriptorTargets()
if err != nil {
t.Fatalf("unexpected error while listing open file descriptors: %v", err)
}

sockets := 0
for _, fd := range fds {
if strings.Contains(fd, "socket") {
sockets++
}
}
return sockets
}