Skip to content

Commit

Permalink
fix (server.Stop): graciously handle various Stop() states.
Browse files Browse the repository at this point in the history
see: #45
  • Loading branch information
jimlambrt committed Jun 21, 2023
1 parent 2ad3888 commit 74e9b22
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 16 deletions.
27 changes: 19 additions & 8 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,17 +164,28 @@ func (s *Server) Stop() error {
const op = "gldap.(Server).Stop"
s.mu.RLock()
defer s.mu.RUnlock()
if s.listener == nil {
return fmt.Errorf("%s: no listener: %w", op, ErrInvalidState)

s.logger.Debug("shutting down")
if s.listener == nil && s.shutdownCancel == nil {
s.logger.Debug("nothing to do for shutdown")
return nil
}
if s.shutdownCancel == nil {
return fmt.Errorf("%s: no shutdown context cancel func: %w", op, ErrInvalidState)

if s.listener != nil {
s.logger.Debug("closing listener")
if err := s.listener.Close(); err != nil {
switch {
case !strings.Contains(err.Error(), "use of closed network connection"):
return fmt.Errorf("%s: %w", op, err)
default:
s.logger.Debug("listener already closed")
}
}
}
if err := s.listener.Close(); err != nil {
return fmt.Errorf("%s: %w", op, err)
if s.shutdownCancel != nil {
s.logger.Debug("shutdown cancel func")
s.shutdownCancel()
}
s.logger.Debug("shutting down")
s.shutdownCancel()
s.logger.Debug("waiting on connections to close")
s.connWg.Wait()
s.logger.Debug("stopped")
Expand Down
56 changes: 48 additions & 8 deletions server_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,27 @@ package gldap

import (
"context"
"errors"
"fmt"
"net"
"os"
"strconv"
"testing"

"github.com/hashicorp/go-hclog"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestServer_Stop(t *testing.T) {
t.Parallel()
var testLogger hclog.Logger
if ok, _ := strconv.ParseBool(os.Getenv("DEBUG")); ok {
testLogger = hclog.New(&hclog.LoggerOptions{
Name: "TestServer_Run-logger",
Level: hclog.Debug,
})
}
tests := []struct {
name string
server *Server
Expand All @@ -21,32 +32,40 @@ func TestServer_Stop(t *testing.T) {
{
name: "missing-listener",
server: func() *Server {
s, err := NewServer()
s, err := NewServer(WithLogger(testLogger))
require.NoError(t, err)
s.mu.Lock()
defer s.mu.Unlock()
s.listener = nil
return s
}(),
wantErr: true,
wantErrContains: "no listener",
},
{
name: "missing-cancel",
server: func() *Server {
p := freePort(t)
l, err := net.Listen("tcp", fmt.Sprintf(":%d", p))
require.NoError(t, err)
s, err := NewServer()
s, err := NewServer(WithLogger(testLogger))
require.NoError(t, err)
s.mu.Lock()
defer s.mu.Unlock()
s.listener = l
s.shutdownCancel = nil
return s
}(),
wantErr: true,
wantErrContains: "no shutdown context cancel func",
},
{
name: "nothing-to-do",
server: func() *Server {
s, err := NewServer(WithLogger(testLogger))
require.NoError(t, err)
s.mu.Lock()
defer s.mu.Unlock()
s.listener = nil
s.shutdownCancel = nil
return s
}(),
},
{
name: "listener-closed",
Expand All @@ -55,7 +74,7 @@ func TestServer_Stop(t *testing.T) {
p := freePort(t)
l, err := net.Listen("tcp", fmt.Sprintf(":%d", p))
require.NoError(t, err)
s, err := NewServer()
s, err := NewServer(WithLogger(testLogger))
require.NoError(t, err)
s.mu.Lock()
defer s.mu.Unlock()
Expand All @@ -64,8 +83,21 @@ func TestServer_Stop(t *testing.T) {
l.Close()
return s
}(),
},
{
name: "listener-close-err",
server: func() *Server {
_, cancel := context.WithCancel(context.Background())
s, err := NewServer(WithLogger(testLogger))
require.NoError(t, err)
s.mu.Lock()
defer s.mu.Unlock()
s.listener = &mockListener{}
s.shutdownCancel = cancel
return s
}(),
wantErr: true,
wantErrContains: "use of closed network connection",
wantErrContains: "mockListener.Close error",
},
}
for _, tc := range tests {
Expand All @@ -83,3 +115,11 @@ func TestServer_Stop(t *testing.T) {
})
}
}

type mockListener struct {
net.Listener
}

func (*mockListener) Close() error {
return errors.New("mockListener.Close error")
}

0 comments on commit 74e9b22

Please sign in to comment.