From 14a30fb6a797683a18a89c86b712416ed11012cb Mon Sep 17 00:00:00 2001 From: pugnack Date: Sun, 27 Apr 2025 15:37:00 +0500 Subject: [PATCH] fix panic on shutdown caused by double channel close (#208) --- service.go | 20 +++++++++++++++++--- service_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/service.go b/service.go index b5b053f8..fb736baa 100644 --- a/service.go +++ b/service.go @@ -104,6 +104,7 @@ type service struct { done chan struct{} opts Options sync.RWMutex + stopped bool } // NewService creates and returns a new Service based on the packages within. @@ -429,7 +430,7 @@ func (s *service) Stop() error { } } - close(s.done) + s.notifyShutdown() return nil } @@ -453,10 +454,23 @@ func (s *service) Run() error { return err } - // wait on context cancel <-s.done - return s.Stop() + return nil +} + +// notifyShutdown marks the service as stopped and closes the done channel. +// It ensures the channel is closed only once, preventing multiple closures. +func (s *service) notifyShutdown() { + s.Lock() + if s.stopped { + s.Unlock() + return + } + s.stopped = true + s.Unlock() + + close(s.done) } type Namer interface { diff --git a/service_test.go b/service_test.go index 47df5dbe..5192a517 100644 --- a/service_test.go +++ b/service_test.go @@ -4,7 +4,9 @@ import ( "context" "reflect" "testing" + "time" + "github.com/stretchr/testify/require" "go.unistack.org/micro/v3/broker" "go.unistack.org/micro/v3/client" "go.unistack.org/micro/v3/config" @@ -773,3 +775,41 @@ func Test_getNameIndex(t *testing.T) { } } */ + +func TestServiceShutdown(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("service shutdown failed: %v", r) + } + }() + + s, ok := NewService().(*service) + require.NotNil(t, s) + require.True(t, ok) + + require.NoError(t, s.Start()) + require.False(t, s.stopped) + + require.NoError(t, s.Stop()) + require.True(t, s.stopped) +} + +func TestServiceMultipleShutdowns(t *testing.T) { + defer func() { + if r := recover(); r != nil { + t.Fatalf("service shutdown failed: %v", r) + } + }() + + s := NewService() + + go func() { + time.Sleep(10 * time.Millisecond) + // first call + require.NoError(t, s.Stop()) + // duplicate call + require.NoError(t, s.Stop()) + }() + + require.NoError(t, s.Run()) +}