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

Data race on onCancelWasCalled #96

Open
bradfitz opened this issue Nov 23, 2021 · 2 comments
Open

Data race on onCancelWasCalled #96

bradfitz opened this issue Nov 23, 2021 · 2 comments

Comments

@bradfitz
Copy link

I got an automated PR telling us to update from pgconn 1.10.0 to 1.10.1 which gave us a helpful diff link:

v1.10.0...v1.10.1

But it looks like the locking on onCancelWasCalled is wrong. The grouping on line 17 below the mutex on line 15 suggests that onCancelWasCalled is guarded by lock, but on line 50, it's assigned without holding the lock. (line 50 is in a goroutine, so the lock/defer Unlock on lines 35/36 don't affect it)

@jackc
Copy link
Owner

jackc commented Nov 23, 2021

The grouping on line 17 is a bit confusing. The mutex guards the initialization of onCancelWasCalled on line 42. But the assignment on line 50 and the read on line 68 are synchronized by unwatchChan.

Perhaps I'm mistaken and there is a race, but I think the tests cover all the orders Watch, Unwatch, and context cancellation could occur and I can't get the race detector to fail.

@bradfitz
Copy link
Author

Sorry, I should've been more explicit that I didn't actually see a data race. I never ran the code. I just read the diff and was suspicious.

Thanks for looking!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants