Skip to content

Commit 71eea82

Browse files
Saad Karimsykesm
Saad Karim
authored andcommitted
[FAB-9130] fix race on metrics.RootScope
Introduced lock around RootScope. Change-Id: I85c5289c369c9a75c6b03e88a8fa76428aa79408 Signed-off-by: Saad Karim <skarim@us.ibm.com> Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>
1 parent 810a43b commit 71eea82

File tree

2 files changed

+26
-14
lines changed

2 files changed

+26
-14
lines changed

common/metrics/server.go

+22-10
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ package metrics
99
import (
1010
"fmt"
1111
"sync"
12-
"sync/atomic"
1312
"time"
1413

1514
"github.com/spf13/viper"
@@ -31,7 +30,8 @@ const (
3130

3231
var RootScope Scope
3332
var once sync.Once
34-
var started uint32
33+
var rootScopeMutex = &sync.Mutex{}
34+
var running bool
3535

3636
// NewOpts create metrics options based config file
3737
func NewOpts() Opts {
@@ -84,21 +84,33 @@ func Init(opts Opts) (err error) {
8484

8585
//Start starts metrics server
8686
func Start() error {
87-
if atomic.CompareAndSwapUint32(&started, 0, 1) {
88-
return RootScope.Start()
87+
rootScopeMutex.Lock()
88+
defer rootScopeMutex.Unlock()
89+
if running {
90+
return nil
8991
}
90-
return nil
92+
running = true
93+
return RootScope.Start()
9194
}
9295

9396
//Shutdown closes underlying resources used by metrics server
9497
func Shutdown() error {
95-
if atomic.CompareAndSwapUint32(&started, 1, 0) {
96-
err := RootScope.Close()
97-
RootScope = nil
98-
return err
98+
rootScopeMutex.Lock()
99+
defer rootScopeMutex.Unlock()
100+
if !running {
101+
return nil
99102
}
100103

101-
return nil
104+
err := RootScope.Close()
105+
RootScope = nil
106+
running = false
107+
return err
108+
}
109+
110+
func isRunning() bool {
111+
rootScopeMutex.Lock()
112+
defer rootScopeMutex.Unlock()
113+
return running
102114
}
103115

104116
type StatsdReporterOpts struct {

common/metrics/server_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ package metrics
99
import (
1010
"fmt"
1111
"strings"
12-
"sync/atomic"
1312
"testing"
1413
"time"
1514

1615
"github.com/hyperledger/fabric/core/config/configtest"
16+
. "github.com/onsi/gomega"
1717
"github.com/spf13/viper"
1818
"github.com/stretchr/testify/assert"
1919
)
@@ -155,6 +155,7 @@ func TestStartInvalidReporter(t *testing.T) {
155155

156156
func TestStartAndClose(t *testing.T) {
157157
t.Parallel()
158+
gt := NewGomegaWithT(t)
158159
defer Shutdown()
159160
opts := Opts{
160161
Enabled: true,
@@ -166,10 +167,9 @@ func TestStartAndClose(t *testing.T) {
166167
FlushBytes: 512,
167168
}}
168169
Init(opts)
169-
go Start()
170-
time.Sleep(1 * time.Second)
171170
assert.NotNil(t, RootScope)
172-
assert.Equal(t, uint32(1), atomic.LoadUint32(&started))
171+
go Start()
172+
gt.Eventually(isRunning).Should(BeTrue())
173173
}
174174

175175
func TestNoOpScopeMetrics(t *testing.T) {

0 commit comments

Comments
 (0)