From 76f1109485b212ed66db45dd7747373f2ec0abeb Mon Sep 17 00:00:00 2001 From: Abhijeet Rastogi Date: Fri, 13 Oct 2023 07:01:15 -0700 Subject: [PATCH] MINOR: mock: add locks to avoid data race Multiple goroutines manage the state of `running` and `responses`, locks should be used to avoid data race. Sample data race:- ``` === RUN TestSingleRuntime_DeleteCrtListEntry === RUN TestSingleRuntime_DeleteCrtListEntry/delete_crt-list_entries_of_crt-list,_should_return_no_error === RUN TestSingleRuntime_DeleteCrtListEntry/delete_crt-list_entries_of_crt-list,_should_return_no_error#01 ================== WARNING: DATA RACE Write at 0x00c000372648 by goroutine 13: github.com/haproxytech/client-native/v5/runtime.(*HAProxyMock).SetResponses() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/haproxy_mock.go:58 +0x9c github.com/haproxytech/client-native/v5/runtime.TestSingleRuntime_DeleteCrtListEntry.func1() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/crt-lists_test.go:413 +0x4a testing.tRunner() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1595 +0x238 testing.(*T).Run.func1() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1648 +0x44 Previous read at 0x00c000372648 by goroutine 11: github.com/haproxytech/client-native/v5/runtime.(*HAProxyMock).handleConnection.func1() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/haproxy_mock.go:84 +0x53d Goroutine 13 (running) created at: testing.(*T).Run() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1648 +0x82a github.com/haproxytech/client-native/v5/runtime.TestSingleRuntime_DeleteCrtListEntry() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/crt-lists_test.go:412 +0x365 testing.tRunner() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1595 +0x238 testing.(*T).Run.func1() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1648 +0x44 Goroutine 11 (finished) created at: github.com/haproxytech/client-native/v5/runtime.(*HAProxyMock).handleConnection() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/haproxy_mock.go:62 +0xe4 github.com/haproxytech/client-native/v5/runtime.(*HAProxyMock).Start.func1() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/haproxy_mock.go:51 +0x36 ================== testing.go:1465: race detected during execution of test ================== WARNING: DATA RACE Write at 0x00c000372640 by goroutine 7: github.com/haproxytech/client-native/v5/runtime.(*HAProxyMock).Stop() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/haproxy_mock.go:35 +0x2f github.com/haproxytech/client-native/v5/runtime.TestSingleRuntime_DeleteCrtListEntry.func2() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/crt-lists_test.go:362 +0x33 runtime.deferreturn() /usr/local/Cellar/go/1.21.2/libexec/src/runtime/panic.go:477 +0x30 testing.tRunner() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1595 +0x238 testing.(*T).Run.func1() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1648 +0x44 Previous read at 0x00c000372640 by goroutine 8: github.com/haproxytech/client-native/v5/runtime.(*HAProxyMock).Start.func1() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/haproxy_mock.go:43 +0x49 Goroutine 7 (running) created at: testing.(*T).Run() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1648 +0x82a testing.runTests.func1() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:2054 +0x84 testing.tRunner() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1595 +0x238 testing.runTests() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:2052 +0x896 testing.(*M).Run() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1925 +0xb57 main.main() _testmain.go:91 +0x2bd Goroutine 8 (running) created at: github.com/haproxytech/client-native/v5/runtime.(*HAProxyMock).Start() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/haproxy_mock.go:41 +0xa9 github.com/haproxytech/client-native/v5/runtime.TestSingleRuntime_DeleteCrtListEntry() /Users/arastogi/Playground/client-native-haproxy/upstream/runtime/crt-lists_test.go:361 +0x48 testing.tRunner() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1595 +0x238 testing.(*T).Run.func1() /usr/local/Cellar/go/1.21.2/libexec/src/testing/testing.go:1648 +0x44 ================== ``` --- runtime/haproxy_mock.go | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/runtime/haproxy_mock.go b/runtime/haproxy_mock.go index 8e85d1bb..3255d9cb 100644 --- a/runtime/haproxy_mock.go +++ b/runtime/haproxy_mock.go @@ -7,6 +7,7 @@ import ( "net" "os" "strings" + "sync" "testing" ) @@ -16,6 +17,8 @@ type HAProxyMock struct { running bool responses map[string]string t *testing.T + + mu sync.RWMutex } // NewHAProxyMock - create new haproxy mock @@ -32,15 +35,15 @@ func NewHAProxyMock(t *testing.T) *HAProxyMock { // Stop - stop mock func (haproxy *HAProxyMock) Stop() { - haproxy.running = false + haproxy.setRunning(false) } // Start - start mock func (haproxy *HAProxyMock) Start() { - haproxy.running = true + haproxy.setRunning(true) go func() { for { - if !haproxy.running { + if !haproxy.getRunning() { return } conn, err := haproxy.Accept() @@ -53,9 +56,34 @@ func (haproxy *HAProxyMock) Start() { }() } -// SetResponses - setting the expected responses +// SetResponses - setting the expected responses, safe for concurrent use func (haproxy *HAProxyMock) SetResponses(responses *map[string]string) { + haproxy.mu.Lock() haproxy.responses = *responses + haproxy.mu.Unlock() +} + +// getResponses gets the responses of the mock, safe for concurrent use +func (haproxy *HAProxyMock) getResponses() map[string]string { + haproxy.mu.RLock() + defer haproxy.mu.RUnlock() + + return haproxy.responses +} + +// setRunning sets the running state of the mock, safe for concurrent use +func (haproxy *HAProxyMock) setRunning(running bool) { + haproxy.mu.Lock() + haproxy.running = running + haproxy.mu.Unlock() +} + +// getRunning gets the running state of the mock, safe for concurrent use +func (haproxy *HAProxyMock) getRunning() bool { + haproxy.mu.RLock() + defer haproxy.mu.RUnlock() + + return haproxy.running } func (haproxy *HAProxyMock) handleConnection(conn net.Conn) { @@ -81,7 +109,7 @@ func (haproxy *HAProxyMock) handleConnection(conn net.Conn) { if len(split) > 0 { s = split[len(split)-1] } - response := haproxy.responses[s] + response := haproxy.getResponses()[s] _, err = w.WriteString(response) if err != nil { haproxy.t.Log(err)