nsqio / go-nsq

The official Go package for NSQ
MIT License
2.59k stars 444 forks source link

go-vet: assignment copies lock value of tls.Config #254

Closed iaburton closed 5 years ago

iaburton commented 5 years ago

While there are some minor linting issues, I noticed go-vet pointing out

assignment copies lock value to conf: crypto/tls.Config contains sync.Once contains sync.Mutex

Here: https://github.com/nsqio/go-nsq/blob/61f49c096d0d767be61e4de06f724e1cb5fd85d7/conn.go#L390

Since the tls.Config is an option in the nsq.Config, you can't be certain it isn't already being used, and therefore synchronization of the Once/Mutex are lost in the copied value. Depending on the minimum Go version this package supports, the solution might be as simple as Cloning the tls.Config with the provided method added in Go1.8 https://golang.org/pkg/crypto/tls/#Config.Clone

ploxiln commented 5 years ago

We don't try to support go < 1.8, Clone() will do fine. Want to open a pull request with that change?

iaburton commented 5 years ago

Done :+1:

ploxiln commented 5 years ago

closing in favor of #255