Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watcher data race #10

Open
banks opened this issue Mar 3, 2020 · 0 comments
Open

Watcher data race #10

banks opened this issue Mar 3, 2020 · 0 comments
Assignees
Labels

Comments

@banks
Copy link

banks commented Mar 3, 2020

https://github.com/haproxytech/haproxy-consul-connect/blob/master/consul/watcher.go

I see a couple of issues in the watcher:

  • removeUpstream is setting a done bool under lock but the startUpstream loop reads that bool while not holding the lock which is a data race.
  • there is no way to close/stop watcher and free goroutines. This is not a big deal in normal usage but our lesson from Consul's TestAgent is that this pattern leads to really inefficient test runs with tons of orphan goroutines by the end etc. I'd recommend plumbing a Stop mechanism or Context through every goroutine spawned right from the start so all resources can be cleaned up in tests and in case you ever need to support managing multiple proxies or decoupling process lifetime and proxy lifetime etc. It's way easier if it's just there already!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants