Skip to content

Commit

Permalink
MINOR: mock: add locks to avoid data race
Browse files Browse the repository at this point in the history 
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
==================
```
  • Loading branch information
@oktalz
Abhijeet Rastogi authored and oktalz committed Nov 3, 2023
1 parent da1a4ea commit 76f1109
Showing 1 changed file with 33 additions and 5 deletions.
38 changes: 33 additions & 5 deletions runtime/haproxy_mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net"
"os"
"strings"
"sync"
"testing"
)

Expand All @@ -16,6 +17,8 @@ type HAProxyMock struct {
running bool
responses map[string]string
t *testing.T

mu sync.RWMutex
}

// NewHAProxyMock - create new haproxy mock
Expand All @@ -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()
Expand All @@ -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) {
Expand All @@ -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)
Expand Down

0 comments on commit 76f1109

Please sign in to comment.