-
Notifications
You must be signed in to change notification settings - Fork 365
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
fix race w/ *Conn.requestTimeout #112
Conversation
conn.go
Outdated
l.requestTimeout = timeout | ||
l.timeoutMutex.Unlock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be simpler to make this an atomic value and do lockless load/store. We already have a shim for < go1.3 using locks
using the atomicValue struct triggered the race during travis build... |
that's concerning... looks like the 1.3 version needs to hold a pointer to the lock |
1.3 was ok, it hit 1.5: https://travis-ci.org/go-ldap/ldap/builds/229842330, the 1.3 failure was a network fetch error while setting up: https://travis-ci.org/go-ldap/ldap/builds/229846728 |
hmm, that's still concerning, but the atomic load/store int64 is nice and clean and sidesteps that issue. go ahead and squash and remove the unused mutex, then LGTM |
this fixes the race seen e.g. in https://travis-ci.org/go-ldap/ldap/jobs/227891683
hmm... https://travis-ci.org/go-ldap/ldap/jobs/230628746 ... what does the race detector find? concurrent r/w access to the *Conn struct's members? |
fixed by #134 |
see https://travis-ci.org/go-ldap/ldap/jobs/227891683
cherry picked from #110