Data race in internal/broadcasters.go

[dlamotte]

Is this a support request?

No.

Describe the bug

https://github.com/launchdarkly/go-server-sdk/blob/v7/internal/broadcasters.go#L70 accesses b.subscribers without guarding the access with a mutex.

To reproduce go test -race You might need -count=100. Our units running with -race -count=100 routinely fail here.

Expected behavior No data races.

Logs ... redacted aspects of our codebase as they are not important.

==================
WARNING: DATA RACE
Read at 0x00c000496180 by goroutine 67:
  github.com/launchdarkly/go-server-sdk/v7/internal.(*Broadcaster[go.shape.struct { Key string }]).HasListeners()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/internal/broadcasters.go:70 +0x5c
  github.com/launchdarkly/go-server-sdk/v7/internal/datasource.(*DataSourceUpdateSinkImpl).Init()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/internal/datasource/data_source_update_sink_impl.go:60 +0x38
  github.com/launchdarkly/go-server-sdk/v7/ldfiledata.(*fileDataSource).reload()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/ldfiledata/file_data_source_impl.go:111 +0x610
  github.com/launchdarkly/go-server-sdk/v7/ldfiledata.(*fileDataSource).reload-fm()
      <autogenerated>:1 +0x34
  github.com/launchdarkly/go-server-sdk/v7/ldfilewatch.(*fileWatcher).run()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/ldfilewatch/watched_file_data_source.go:67 +0x1b4
  github.com/launchdarkly/go-server-sdk/v7/ldfilewatch.WatchFiles.func1()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/ldfilewatch/watched_file_data_source.go:44 +0x40

Previous write at 0x00c000496180 by goroutine 38:
  github.com/launchdarkly/go-server-sdk/v7/internal.(*Broadcaster[go.shape.struct { Key string }]).Close()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/internal/broadcasters.go:92 +0xf4
  github.com/launchdarkly/go-server-sdk/v7.(*LDClient).Close()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/ldclient.go:539 +0x220
...
  testing.(*common).Cleanup.func1()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1169 +0x134
  testing.(*common).runCleanup()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1347 +0x148
  testing.tRunner.func2()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1589 +0x48
  runtime.deferreturn()
      /Users/dlamotte/.goenv/versions/1.21.4/src/runtime/panic.go:477 +0x34
  testing.(*T).Run.func1()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1648 +0x40

Goroutine 67 (running) created at:
  github.com/launchdarkly/go-server-sdk/v7/ldfilewatch.WatchFiles()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/ldfilewatch/watched_file_data_source.go:44 +0x2c0
  github.com/launchdarkly/go-server-sdk/v7/ldfiledata.(*fileDataSource).Start()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/ldfiledata/file_data_source_impl.go:80 +0x200
  github.com/launchdarkly/go-server-sdk/v7.MakeCustomClient()
      /Users/dlamotte/go/1.21.4/pkg/mod/github.com/launchdarkly/go-server-sdk/[email protected]/ldclient.go:292 +0x1aa4
...
  testing.tRunner()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1595 +0x194
  testing.(*T).Run.func1()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1648 +0x40

Goroutine 38 (finished) created at:
  testing.(*T).Run()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1648 +0x5d8
  testing.runTests.func1()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:2054 +0x80
  testing.tRunner()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1595 +0x194
  testing.runTests()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:2052 +0x6d8
  testing.(*M).Run()
      /Users/dlamotte/.goenv/versions/1.21.4/src/testing/testing.go:1925 +0x908
  main.main()
      _testmain.go:57 +0x294
==================

SDK version

github.com/launchdarkly/go-server-sdk/v7 v7.0.0

Language version, developer tools go version go1.21.4 darwin/arm64

OS/platform OSX.

Additional context Impact is that we cannot use -race when running our unit tests to find races. So we have to have two unit test runs until this is fixed which is not ideal as we actually had a data race in our code on top of this code. So, I'd prefer to eliminate this race so we can ensure we don't have races in our code.

[cwaldren-ld]

Hi @dlamotte, thank you for the report. I'll investigate.

[cwaldren-ld]

Can you post your SDK configuration?

[dlamotte]

@cwaldren-ld I think you want this?

import (
...
ld "github.com/launchdarkly/go-server-sdk/v7"
)

	var config ld.Config
	{
		if conf := flags.options.Config; conf != nil {
			config = ld.Config{
				DataSource: ldfiledata.DataSource().
					FilePaths(*conf).
					Reloader(ldfilewatch.WatchFiles),
				Events: ldcomponents.NoEvents(),
			}
		} else {
			config = ld.Config{
				DataSource: ldcomponents.StreamingDataSource().InitialReconnectDelay(5 * time.Second),
				Offline:    false,
			}
		}

		config.DiagnosticOptOut = true
	}

	config.Logging = ldcomponents.Logging().MinLevel(ldlog.Error)

	client, err := ld.MakeCustomClient(apikey, config, 5*time.Second)

It's a partial. Hopefully its enough.

[dlamotte]

Appears fixed by https://github.com/launchdarkly/go-server-sdk/pull/151 (looks like there is another follow-on PR, but best follow those PRs)

[cwaldren-ld]

This has been released as part of https://github.com/launchdarkly/go-server-sdk/releases/tag/v7.4.1