From 194a71a12b693e345158564a7170b4da0cbf0f60 Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Mon, 4 Apr 2022 16:41:55 +0200 Subject: [PATCH] Address data race in pkg/corredor (healthcheck) --- pkg/corredor/healthcheck.go | 11 +++++++---- pkg/corredor/healthcheck_test.go | 16 ++++++++++++++++ pkg/corredor/service.go | 9 +++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 pkg/corredor/healthcheck_test.go diff --git a/pkg/corredor/healthcheck.go b/pkg/corredor/healthcheck.go index 4ab7a2b59..bd10edd00 100644 --- a/pkg/corredor/healthcheck.go +++ b/pkg/corredor/healthcheck.go @@ -3,20 +3,23 @@ package corredor import ( "context" "fmt" + "google.golang.org/grpc/connectivity" ) // Healtcheck for global -func Healthcheck(ctx context.Context) error { - if gCorredor == nil { +func Healthcheck(_ context.Context) error { + svc := Service() + + if svc == nil { return fmt.Errorf("uninitialized") } - if !gCorredor.opt.Enabled { + if !svc.opt.Enabled { return nil } - if state := gCorredor.conn.GetState(); state != connectivity.Ready { + if state := svc.conn.GetState(); state != connectivity.Ready { return fmt.Errorf("connection is %s", state) } diff --git a/pkg/corredor/healthcheck_test.go b/pkg/corredor/healthcheck_test.go new file mode 100644 index 000000000..ee71fcf3a --- /dev/null +++ b/pkg/corredor/healthcheck_test.go @@ -0,0 +1,16 @@ +package corredor + +import ( + "context" + "testing" + + "github.com/cortezaproject/corteza-server/pkg/options" + "go.uber.org/zap" +) + +// tested with +// go test -count 10 -race -run TestDataRace ./pkg/corredor/... +func TestDataRace(t *testing.T) { + go Setup(zap.NewNop(), options.CorredorOpt{}) + go Healthcheck(context.Background()) +} diff --git a/pkg/corredor/service.go b/pkg/corredor/service.go index c2de552d5..59a419e05 100644 --- a/pkg/corredor/service.go +++ b/pkg/corredor/service.go @@ -119,6 +119,9 @@ var ( // Global corredor service gCorredor *service + // Lock for accessing global service + gLock sync.RWMutex + // List of event types that can be used as iteration // initiator // @@ -144,11 +147,17 @@ const ( ) func Service() *service { + gLock.RLock() + defer gLock.RUnlock() + return gCorredor } // Setup start connects to Corredor & initialize service func Setup(logger *zap.Logger, opt options.CorredorOpt) (err error) { + gLock.Lock() + defer gLock.Unlock() + if gCorredor != nil { // Prevent multiple initializations return