samber / do

⚙️ A dependency injection toolkit based on Go 1.18+ Generics.
https://pkg.go.dev/github.com/samber/do
MIT License
1.71k stars 67 forks source link

Data race: unsynchronized read of `Injector.services` in `serviceNotFound` #43

Open dgunay opened 10 months ago

dgunay commented 10 months ago

When a service is not found in Invoke/MustInvoke/InvokeNamed, serviceNotFound is called

func (i *Injector) serviceNotFound(name string) error {
    // @TODO: use the Keys+Map functions from `golang.org/x/exp/maps` as
    // soon as it is released in stdlib.
    servicesNames := keys(i.services)
    servicesNames = mAp(servicesNames, func(name string) string {
        return fmt.Sprintf("`%s`", name)
    })

    return fmt.Errorf("DI: could not find service `%s`, available services: %s", name, strings.Join(servicesNames, ", "))
}

which calls keys(i.services). keys performs a read on i.services but serviceNotFound hasn't acquired the mutex for reading. This trips the race detector in some of my tests.

Solution is probably to just acquire the mutex for reading at the beginning of either serviceNotFound or InvokeNamed.

data race:

WARNING: DATA RACE
Write at 0x00c000f51440 by goroutine 20:
  runtime.mapaccess2_faststr()
      /opt/homebrew/Cellar/go/1.21.1/libexec/src/runtime/map_faststr.go:108 +0x42c
  github.com/samber/do.(*Injector).set()
      /Users/devin/go/pkg/mod/github.com/samber/do@v1.6.0/injector.go:239 +0xa4
  github.com/samber/do.ProvideValue[go.shape.interface { Sleep(time.Duration) }]()
      /Users/devin/go/pkg/mod/github.com/samber/do@v1.6.0/di.go:42 +0x188
  # proprietary code which calls ProvideValue

Previous read at 0x00c000f51440 by goroutine 21:
  github.com/samber/do.keys[go.shape.string,go.shape.interface {}]()
      /Users/devin/go/pkg/mod/github.com/samber/do@v1.6.0/utils.go:14 +0x54
  github.com/samber/do.(*Injector).serviceNotFound()
      /Users/devin/go/pkg/mod/github.com/samber/do@v1.6.0/injector.go:264 +0x34
  github.com/samber/do.InvokeNamed[go.shape.interface { Sleep(time.Duration) }]()
      /Users/devin/go/pkg/mod/github.com/samber/do@v1.6.0/di.go:115 +0x20c
  github.com/samber/do.Invoke[go.shape.interface { Sleep(time.Duration) }]()
      /Users/devin/go/pkg/mod/github.com/samber/do@v1.6.0/di.go:101 +0x6c
# proprietary code which calls Invoke
dgunay commented 9 months ago

For anyone reading this, if you do not want to fork then you can work around the issue by essentially wrapping the library yourself in a RWMutex.

dgunay commented 7 months ago

Maybe fixed by #48 ? I haven't read the PR yet but presumably using sync.Map would fix this.

samber commented 7 months ago

Hi @dgunay

The v1 was not 100% thread-safe. We are working on the matter for v2 (WIP).