search

launchdarkly / go-server-sdk

LaunchDarkly Server-side SDK for Go
Other
41 stars 18 forks source link

Data race in internal/broadcasters.go #102

Closed dlamotte closed 4 months ago

dlamotte commented 7 months ago

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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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/v7@v7.0.0/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 commented 7 months ago

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

cwaldren-ld commented 7 months ago

Can you post your SDK configuration?

dlamotte commented 7 months ago

@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 commented 4 months ago

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 commented 4 months ago

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