ApplicationInsights-Go
ApplicationInsights-Go copied to clipboard
Data race in tests
Hi,
Seems there's a data race. Running go test --race ./appinsights/...
produces the following:
==================
WARNING: DATA RACE
Write at 0x000000cc64e0 by goroutine 27:
github.com/microsoft/ApplicationInsights-Go/appinsights.(*diagnosticsMessageWriter).appendListener()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/diagnostics.go:50 +0xed
github.com/microsoft/ApplicationInsights-Go/appinsights.TestMessageSentToConsumers()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/diagnostics.go:43 +0x1cf
testing.tRunner()
/usr/local/go/src/testing/testing.go:865 +0x163
Previous read at 0x000000cc64e0 by goroutine 17:
github.com/microsoft/ApplicationInsights-Go/appinsights.(*httpTransmitter).Transmit()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/diagnostics.go:87 +0x9d9
github.com/microsoft/ApplicationInsights-Go/appinsights.(*InMemoryChannel).transmitRetry()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/inmemorychannel.go:370 +0x23b
github.com/microsoft/ApplicationInsights-Go/appinsights.(*inMemoryChannelState).send.func1()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/inmemorychannel.go:289 +0xbd
Goroutine 27 (running) created at:
testing.(*T).Run()
/usr/local/go/src/testing/testing.go:916 +0x65a
testing.runTests.func1()
/usr/local/go/src/testing/testing.go:1157 +0xa8
testing.tRunner()
/usr/local/go/src/testing/testing.go:865 +0x163
testing.runTests()
/usr/local/go/src/testing/testing.go:1155 +0x523
testing.(*M).Run()
/usr/local/go/src/testing/testing.go:1072 +0x2eb
main.main()
_testmain.go:136 +0x222
Goroutine 17 (finished) created at:
github.com/microsoft/ApplicationInsights-Go/appinsights.(*inMemoryChannelState).send()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/inmemorychannel.go:287 +0x1d2
github.com/microsoft/ApplicationInsights-Go/appinsights.(*inMemoryChannelState).waitToSend()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/inmemorychannel.go:264 +0x7d4
github.com/microsoft/ApplicationInsights-Go/appinsights.(*inMemoryChannelState).start()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/inmemorychannel.go:210 +0x384
github.com/microsoft/ApplicationInsights-Go/appinsights.(*InMemoryChannel).acceptLoop()
/home/hbarel/go/src/github.com/microsoft/ApplicationInsights-Go/appinsights/inmemorychannel.go:146 +0x4d
==================
--- FAIL: TestMessageSentToConsumers (0.00s)
testing.go:809: race detected during execution of test
FAIL
FAIL github.com/microsoft/ApplicationInsights-Go/appinsights 4.022s
? github.com/microsoft/ApplicationInsights-Go/appinsights/contracts [no test files]
Introducing a writer.lock.Lock()
(and a defer unlock of course) to diagnostics.go:87 in the hasListeners()
function seems to resolve it
I wasn't a fan of locking the diagnostics listeners (it may constitute a race condition, but I don't think errors could be induced from this) when I implemented it, but I think a reader-writer lock would be acceptable.