Skip to content

Commit 01b9d2e

Browse files
justonedev1Michal Tichák
andauthored
Fix race condition in tests (#747)
* [monitoring] fixed racecondition on Run monitoring and Stop * fixup! [monitoring] fixed racecondition on Run monitoring and Stop --------- Co-authored-by: Michal Tichák <michal.tichak@cern.ch>
1 parent 2d0bcb1 commit 01b9d2e

File tree

2 files changed

+14
-24
lines changed

2 files changed

+14
-24
lines changed

common/monitoring/monitoring.go

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"context"
3131
"fmt"
3232
"net/http"
33+
"sync/atomic"
3334
"time"
3435

3536
"github.com/AliceO2Group/Control/common/logger"
@@ -38,8 +39,8 @@ import (
3839
)
3940

4041
var (
41-
// scraping endpoint implementation
42-
server *http.Server
42+
// atomic holder for the HTTP server instance
43+
server atomic.Pointer[http.Server]
4344
// objects to store incoming metrics
4445
metricsInternal *MetricsAggregate
4546
metricsHistogramInternal *MetricsReservoirSampling
@@ -154,33 +155,30 @@ func handleFunc(endpointName string) {
154155
//
155156
// If we attempt send more messages than the size of the buffer, these overflowing messages will be ignored and warning will be logged.
156157
func Run(port uint16, endpointName string) error {
157-
if IsRunning() {
158+
localServer := &http.Server{Addr: fmt.Sprintf(":%d", port)}
159+
// only one Run should initialize and serve
160+
if !server.CompareAndSwap(nil, localServer) {
158161
return nil
159162
}
160-
161163
initChannels()
162-
163164
go eventLoop()
164-
165-
server = &http.Server{Addr: fmt.Sprintf(":%d", port)}
166165
handleFunc(endpointName)
167-
return server.ListenAndServe()
166+
// block until Shutdown is called
167+
return localServer.ListenAndServe()
168168
}
169169

170170
func Stop() {
171-
if !IsRunning() {
171+
localServer := server.Swap(nil)
172+
if localServer == nil {
172173
return
173174
}
174-
175175
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
176176
defer cancel()
177-
server.Shutdown(ctx)
178-
177+
localServer.Shutdown(ctx)
179178
endChannel <- struct{}{}
180179
<-endChannel
181-
server = nil
182180
}
183181

184182
func IsRunning() bool {
185-
return server != nil
183+
return server.Load() != nil
186184
}

common/monitoring/monitoring_test.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,20 +81,12 @@ func TestStartMultipleStop(t *testing.T) {
8181
Stop()
8282
}
8383

84-
func cleaningUpAfterTest() {
85-
Stop()
86-
}
87-
88-
func initTest() {
89-
go Run(12345, "notimportant")
90-
}
91-
9284
// decorator function that properly inits and cleans after higher level test of Monitoring package
9385
func testFunction(t *testing.T, testToRun func(*testing.T)) {
94-
initTest()
86+
go Run(12345, "notimportant")
9587
isRunningWithTimeout(t, time.Second)
9688
testToRun(t)
97-
cleaningUpAfterTest()
89+
Stop()
9890
}
9991

10092
func TestSendingSingleMetric(t *testing.T) {

0 commit comments

Comments
 (0)