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

Potential cache/throttle race conditions in Connector._resolve_host with traces #4013

Closed
ojii opened this issue Aug 29, 2019 · 0 comments · Fixed by #4014
Closed

Potential cache/throttle race conditions in Connector._resolve_host with traces #4013

ojii opened this issue Aug 29, 2019 · 0 comments · Fixed by #4014

Comments

@ojii
Copy link
Contributor

ojii commented Aug 29, 2019

Long story short

If traces are used in TCPConnector._resolve_host the context switch introduced by awaiting their trace API calls can lead to various race conditions.

For example the DNS cache hit test may evaluate True but by the time traces is done it may already be gone again.

The same is true for the throttling logic where between checking that the key is already throttled and then accessing that key the key may already be popped.

Similarly in the opposite case of the above, with traces enabled, the code might override an already set throttled event

Steps to reproduce

See my branch

In the tests I mess with the caches from the traces directly to simulate the effects for simplicity.

@helpr helpr bot added pr-merged and removed pr-rejected labels Aug 31, 2019
asvetlov pushed a commit that referenced this issue Aug 31, 2019
(cherry picked from commit 010caab)

Co-authored-by: Jonas Obrist <[email protected]>
asvetlov added a commit that referenced this issue Aug 31, 2019
(cherry picked from commit 010caab)

Co-authored-by: Jonas Obrist <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant