diff --git a/pkg/network/server.go b/pkg/network/server.go index 0e919cbdd..b97cf2dd1 100644 --- a/pkg/network/server.go +++ b/pkg/network/server.go @@ -138,6 +138,9 @@ type ( stateSync StateSync log *zap.Logger + + // started used to Start and Shutdown server only once. + started atomic.Bool } peerDrop struct { @@ -262,8 +265,12 @@ func (s *Server) ID() uint32 { } // Start will start the server and its underlying transport. Calling it twice -// is an error. Caller should wait for Start to finish for normal server operation. +// is a no-op. Caller should wait for Start to finish for normal server operation. func (s *Server) Start() { + if !s.started.CompareAndSwap(false, true) { + s.log.Info("node server already started") + return + } s.log.Info("node started", zap.Uint32("blockHeight", s.chain.BlockHeight()), zap.Uint32("headerHeight", s.chain.HeaderHeight())) @@ -288,9 +295,12 @@ func (s *Server) Start() { go s.run() } -// Shutdown disconnects all peers and stops listening. Calling it twice is an error, +// Shutdown disconnects all peers and stops listening. Calling it twice is a no-op, // once stopped the same intance of the Server can't be started again by calling Start. func (s *Server) Shutdown() { + if !s.started.CompareAndSwap(true, false) { + return + } s.log.Info("shutting down server", zap.Int("peers", s.PeerCount())) for _, tr := range s.transports { tr.Close() diff --git a/pkg/network/server_test.go b/pkg/network/server_test.go index 7dbce881d..c1b5d96dc 100644 --- a/pkg/network/server_test.go +++ b/pkg/network/server_test.go @@ -96,10 +96,12 @@ func TestServerStartAndShutdown(t *testing.T) { require.Eventually(t, func() bool { return 1 == s.PeerCount() }, time.Second, time.Millisecond*10) assert.True(t, s.transports[0].(*fakeTransp).started.Load()) + require.True(t, s.started.Load()) assert.Nil(t, s.txCallback) s.Shutdown() + require.False(t, s.started.Load()) require.True(t, s.transports[0].(*fakeTransp).closed.Load()) err, ok := p.droppedWith.Load().(error) require.True(t, ok) @@ -115,11 +117,34 @@ func TestServerStartAndShutdown(t *testing.T) { s.register <- p assert.True(t, s.services["fake"].(*fakeConsensus).started.Load()) + require.True(t, s.started.Load()) s.Shutdown() + require.False(t, s.started.Load()) require.True(t, s.services["fake"].(*fakeConsensus).stopped.Load()) }) + t.Run("double start", func(t *testing.T) { + s := newTestServer(t, ServerConfig{}) + startWithCleanup(t, s) + + // Attempt to start the server again. + s.Start() + + require.True(t, s.started.Load(), "server should still be marked as started after second Start call") + }) + t.Run("double shutdown", func(t *testing.T) { + s := newTestServer(t, ServerConfig{}) + s.Start() + require.True(t, s.started.Load(), "server should still be marked as started after second Start call") + s.Shutdown() + + require.False(t, s.started.Load(), "server should be marked as not started after second Shutdown call") + // Attempt to shutdown the server again. + s.Shutdown() + // Verify the server state remains unchanged and is still considered shutdown. + require.False(t, s.started.Load(), "server should remain shutdown after second call") + }) } func TestServerRegisterPeer(t *testing.T) {