[dogstatsd] Fix race condition in _flush_buffer#602
[dogstatsd] Fix race condition in _flush_buffer#602NorthIsUp wants to merge 3 commits intoDataDog:masterfrom
Conversation
|
@jd does this look better? |
jd
left a comment
jd
left a comment
There was a problem hiding this comment.
I don't think you're going to solve the problem correctly that way, it's a little more complicated that just adding with lock TBH and needs redesign of the API IMHO.
| self._send_to_server("\n".join(self.buffer)) | ||
| self._current_buffer_total_size = 0 | ||
| self.buffer = [] | ||
| with self.lock: |
There was a problem hiding this comment.
This lock is used for socket handling, it's not a great idea to re-use it for another purpose.
This also only solves a potential race around _flush_buffer, while the original bug report mentioned open_buffer as it's the first one to be the issue.
|
@NorthIsUp are you hitting an issue in particular for working on this? |
|
This issue has been automatically marked as stale because it has not had activity in the last 30 days. |
|
Closing since this was fixed via this PR now |
3 participants
What does this PR do?
fixes bug #439
Description of the Change
copy the global buffer to a local buffer so we can't accidentally flush it twice.
Alternate Designs
self.buffercould be a queue, but that has other issues, this approach should be good enough without the complexityVerification Process
existing tests should cover this
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.