From ed6c3ba082bdbc82284c198d93ca5f07ad9900dd Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Fri, 13 Sep 2024 16:02:32 +0300 Subject: [PATCH 1/2] server_test: add Serve()/Shutdown() race test. Signed-off-by: Krisztian Litkey --- server_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/server_test.go b/server_test.go index cf34986d6..4a1561df8 100644 --- a/server_test.go +++ b/server_test.go @@ -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() From c4d96d55ad9c4f4cf6036c70a5b18ba80655d648 Mon Sep 17 00:00:00 2001 From: Krisztian Litkey Date: Fri, 13 Sep 2024 15:06:23 +0300 Subject: [PATCH 2/2] server: fix Serve() vs. immediate Shutdown() race. Fix a race where an asynchronous server.Serve() invoked in a a goroutine races with an almost immediate server.Shutdown(). If Shutdown() finishes its locked closing of listeners before Serve() gets around to add the new one, Serve will sit stuck forever in l.Accept(), unless the caller closes the listener in addition to Shutdown(). This is probably almost impossible to trigger in real life, but some of the unit tests, which run the server and client in the same process, occasionally do trigger this. Then, if the test tries to verify a final ErrServerClosed error from Serve() after Shutdown() it gets stuck forever. Signed-off-by: Krisztian Litkey --- server.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/server.go b/server.go index 26419831d..bb71de677 100644 --- a/server.go +++ b/server.go @@ -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 @@ -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{}{} }