Skip to content

Commit

Permalink
fix: Don't close the listener when closing the server
Browse files Browse the repository at this point in the history
When calling gluon.Serve(ctx, net.Listener), we take in a listener.
We shouldn't close this when closing gluon as we don't own this listener.
The surrounding caller who calls gluon.Serve(...) should be responsible for
closing it, to avoid double-close errors.
  • Loading branch information
jameshoulahan committed Sep 9, 2022
1 parent 0965966 commit fdc36d5
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 6 deletions.
6 changes: 5 additions & 1 deletion benchmarks/gluon_bench/imap_benchmarks/server/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ func (l *LocalServer) Address() net.Addr {
}

func (l *LocalServer) Close(ctx context.Context) error {
return l.server.Close(ctx)
if err := l.server.Close(ctx); err != nil {
return err
}

return l.listener.Close()
}

type LocalServerBuilder struct{}
Expand Down
4 changes: 4 additions & 0 deletions demo/demo.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ func main() {
for err := range server.GetErrorCh() {
logrus.WithError(err).Error("Error while serving")
}

if err := listener.Close(); err != nil {
logrus.WithError(err).Error("Failed to close listener")
}
}

func addUser(ctx context.Context, server *gluon.Server, addresses []string, password string) error {
Expand Down
6 changes: 1 addition & 5 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,7 @@ func (s *Server) Serve(ctx context.Context, l net.Listener) error {
Addr: l.Addr(),
})

connCh := newConnCh(l)
defer l.Close()

s.serve(ctx, connCh)
s.serve(ctx, newConnCh(l))
})

return nil
Expand Down Expand Up @@ -231,7 +228,6 @@ func (s *Server) GetUserDataPath(userID string) (string, error) {
}

// Close closes the server.
// It firstly closes all TCP listeners then closes the backend.
func (s *Server) Close(ctx context.Context) error {
ctx = reporter.NewContextWithReporter(ctx, s.reporter)

Expand Down
1 change: 1 addition & 0 deletions tests/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ func runServer(tb testing.TB, options *serverOptions, tests func(session *testSe
// Expect the server to shut down successfully when closed.
require.NoError(tb, server.Close(ctx))
require.NoError(tb, <-server.GetErrorCh())
require.NoError(tb, listener.Close())
}

// runServerWithPaths initializes and starts the mailserver.
Expand Down

0 comments on commit fdc36d5

Please sign in to comment.