Skip to content

Commit

Permalink
Merge pull request #175 from klihub/fixes/serve-listen-shutdown-race
Browse files Browse the repository at this point in the history
server: fix a Serve() vs. (immediate) Shutdown() race
  • Loading branch information
AkihiroSuda authored Oct 29, 2024
2 parents bcc40a4 + c4d96d5 commit b71d9de
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 4 deletions.
15 changes: 11 additions & 4 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,18 @@ func (s *Server) RegisterService(name string, desc *ServiceDesc) {
}

func (s *Server) Serve(ctx context.Context, l net.Listener) error {
s.addListener(l)
s.mu.Lock()
s.addListenerLocked(l)
defer s.closeListener(l)

select {
case <-s.done:
s.mu.Unlock()
return ErrServerClosed
default:
}
s.mu.Unlock()

var (
backoff time.Duration
handshaker = s.config.handshaker
Expand Down Expand Up @@ -188,9 +197,7 @@ func (s *Server) Close() error {
return err
}

func (s *Server) addListener(l net.Listener) {
s.mu.Lock()
defer s.mu.Unlock()
func (s *Server) addListenerLocked(l net.Listener) {
s.listeners[l] = struct{}{}
}

Expand Down
30 changes: 30 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,36 @@ func TestServerClose(t *testing.T) {
checkServerShutdown(t, server)
}

func TestImmediateServerShutdown(t *testing.T) {
var (
ctx = context.Background()
server = mustServer(t)(NewServer())
addr, listener = newTestListener(t)
errs = make(chan error, 1)
_, cleanup = newTestClient(t, addr)
)
defer cleanup()
defer listener.Close()
go func() {
time.Sleep(1 * time.Millisecond)
errs <- server.Serve(ctx, listener)
}()

registerTestingService(server, &testingServer{})

if err := server.Shutdown(ctx); err != nil {
t.Fatal(err)
}
select {
case err := <-errs:
if err != ErrServerClosed {
t.Fatal(err)
}
case <-time.After(2 * time.Second):
t.Fatal("retreiving error from server.Shutdown() timed out")
}
}

func TestOversizeCall(t *testing.T) {
var (
ctx = context.Background()
Expand Down

0 comments on commit b71d9de

Please sign in to comment.