-
Notifications
You must be signed in to change notification settings - Fork 520
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: Fix deadlock in transport due to GC running #814
Changes from 1 commit
38324cf
6c0832b
484ba67
888a921
073e8bf
af82caf
c7fc365
74673fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,222 @@ | ||
""" | ||
A fork of Python 3.6's stdlib queue with Lock swapped out for RLock to avoid a | ||
deadlock while garbage collecting. | ||
|
||
See | ||
https://codewithoutrules.com/2017/08/16/concurrency-python/ | ||
https://bugs.python.org/issue14976 | ||
https://github.com/sqlalchemy/sqlalchemy/blob/4eb747b61f0c1b1c25bdee3856d7195d10a0c227/lib/sqlalchemy/queue.py#L1 | ||
|
||
We also vendor the code to evade eventlet's broken monkeypatching, see | ||
https://github.com/getsentry/sentry-python/pull/484 | ||
""" | ||
|
||
try: | ||
import threading | ||
except ImportError: | ||
import dummy_threading as threading | ||
from collections import deque | ||
from heapq import heappush, heappop | ||
from time import monotonic as time | ||
|
||
__all__ = ['Empty', 'Full', 'Queue', 'PriorityQueue', 'LifoQueue'] | ||
|
||
class Empty(Exception): | ||
'Exception raised by Queue.get(block=0)/get_nowait().' | ||
pass | ||
|
||
class Full(Exception): | ||
'Exception raised by Queue.put(block=0)/put_nowait().' | ||
pass | ||
|
||
class Queue(object): | ||
'''Create a queue object with a given maximum size. | ||
|
||
If maxsize is <= 0, the queue size is infinite. | ||
''' | ||
|
||
def __init__(self, maxsize=0): | ||
self.maxsize = maxsize | ||
self._init(maxsize) | ||
|
||
# mutex must be held whenever the queue is mutating. All methods | ||
# that acquire mutex must release it before returning. mutex | ||
# is shared between the three conditions, so acquiring and | ||
# releasing the conditions also acquires and releases mutex. | ||
self.mutex = threading.RLock() | ||
|
||
# Notify not_empty whenever an item is added to the queue; a | ||
# thread waiting to get is notified then. | ||
self.not_empty = threading.Condition(self.mutex) | ||
|
||
# Notify not_full whenever an item is removed from the queue; | ||
# a thread waiting to put is notified then. | ||
self.not_full = threading.Condition(self.mutex) | ||
|
||
# Notify all_tasks_done whenever the number of unfinished tasks | ||
# drops to zero; thread waiting to join() is notified to resume | ||
self.all_tasks_done = threading.Condition(self.mutex) | ||
self.unfinished_tasks = 0 | ||
|
||
def task_done(self): | ||
'''Indicate that a formerly enqueued task is complete. | ||
|
||
Used by Queue consumer threads. For each get() used to fetch a task, | ||
a subsequent call to task_done() tells the queue that the processing | ||
on the task is complete. | ||
|
||
If a join() is currently blocking, it will resume when all items | ||
have been processed (meaning that a task_done() call was received | ||
for every item that had been put() into the queue). | ||
|
||
Raises a ValueError if called more times than there were items | ||
placed in the queue. | ||
''' | ||
with self.all_tasks_done: | ||
unfinished = self.unfinished_tasks - 1 | ||
if unfinished <= 0: | ||
if unfinished < 0: | ||
raise ValueError('task_done() called too many times') | ||
self.all_tasks_done.notify_all() | ||
self.unfinished_tasks = unfinished | ||
|
||
def join(self): | ||
'''Blocks until all items in the Queue have been gotten and processed. | ||
|
||
The count of unfinished tasks goes up whenever an item is added to the | ||
queue. The count goes down whenever a consumer thread calls task_done() | ||
to indicate the item was retrieved and all work on it is complete. | ||
|
||
When the count of unfinished tasks drops to zero, join() unblocks. | ||
''' | ||
with self.all_tasks_done: | ||
while self.unfinished_tasks: | ||
self.all_tasks_done.wait() | ||
|
||
def qsize(self): | ||
'''Return the approximate size of the queue (not reliable!).''' | ||
with self.mutex: | ||
return self._qsize() | ||
|
||
def empty(self): | ||
'''Return True if the queue is empty, False otherwise (not reliable!). | ||
|
||
This method is likely to be removed at some point. Use qsize() == 0 | ||
as a direct substitute, but be aware that either approach risks a race | ||
condition where a queue can grow before the result of empty() or | ||
qsize() can be used. | ||
|
||
To create code that needs to wait for all queued tasks to be | ||
completed, the preferred technique is to use the join() method. | ||
''' | ||
with self.mutex: | ||
return not self._qsize() | ||
|
||
def full(self): | ||
'''Return True if the queue is full, False otherwise (not reliable!). | ||
|
||
This method is likely to be removed at some point. Use qsize() >= n | ||
as a direct substitute, but be aware that either approach risks a race | ||
condition where a queue can shrink before the result of full() or | ||
qsize() can be used. | ||
''' | ||
with self.mutex: | ||
return 0 < self.maxsize <= self._qsize() | ||
|
||
def put(self, item, block=True, timeout=None): | ||
'''Put an item into the queue. | ||
|
||
If optional args 'block' is true and 'timeout' is None (the default), | ||
block if necessary until a free slot is available. If 'timeout' is | ||
a non-negative number, it blocks at most 'timeout' seconds and raises | ||
the Full exception if no free slot was available within that time. | ||
Otherwise ('block' is false), put an item on the queue if a free slot | ||
is immediately available, else raise the Full exception ('timeout' | ||
is ignored in that case). | ||
''' | ||
with self.not_full: | ||
if self.maxsize > 0: | ||
if not block: | ||
if self._qsize() >= self.maxsize: | ||
raise Full() | ||
elif timeout is None: | ||
while self._qsize() >= self.maxsize: | ||
self.not_full.wait() | ||
elif timeout < 0: | ||
raise ValueError("'timeout' must be a non-negative number") | ||
else: | ||
endtime = time() + timeout | ||
while self._qsize() >= self.maxsize: | ||
remaining = endtime - time() | ||
if remaining <= 0.0: | ||
raise Full | ||
self.not_full.wait(remaining) | ||
self._put(item) | ||
self.unfinished_tasks += 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This
Importantly here Is that acceptable for us? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think they'd notice but I think we should figure out hwo to get rid of this race There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you decrement? Could use two counters I guess, and the value is the subtraction of both. I am no longer of the opinion that we need to fix though. |
||
self.not_empty.notify() | ||
|
||
def get(self, block=True, timeout=None): | ||
'''Remove and return an item from the queue. | ||
|
||
If optional args 'block' is true and 'timeout' is None (the default), | ||
block if necessary until an item is available. If 'timeout' is | ||
a non-negative number, it blocks at most 'timeout' seconds and raises | ||
the Empty exception if no item was available within that time. | ||
Otherwise ('block' is false), return an item if one is immediately | ||
available, else raise the Empty exception ('timeout' is ignored | ||
in that case). | ||
''' | ||
with self.not_empty: | ||
if not block: | ||
if not self._qsize(): | ||
raise Empty() | ||
elif timeout is None: | ||
while not self._qsize(): | ||
self.not_empty.wait() | ||
elif timeout < 0: | ||
raise ValueError("'timeout' must be a non-negative number") | ||
else: | ||
endtime = time() + timeout | ||
while not self._qsize(): | ||
remaining = endtime - time() | ||
if remaining <= 0.0: | ||
raise Empty() | ||
self.not_empty.wait(remaining) | ||
item = self._get() | ||
self.not_full.notify() | ||
return item | ||
|
||
def put_nowait(self, item): | ||
'''Put an item into the queue without blocking. | ||
|
||
Only enqueue the item if a free slot is immediately available. | ||
Otherwise raise the Full exception. | ||
''' | ||
return self.put(item, block=False) | ||
|
||
def get_nowait(self): | ||
'''Remove and return an item from the queue without blocking. | ||
|
||
Only get an item if one is immediately available. Otherwise | ||
raise the Empty exception. | ||
''' | ||
return self.get(block=False) | ||
|
||
# Override these methods to implement other queue organizations | ||
# (e.g. stack or priority queue). | ||
# These will only be called with appropriate locks held | ||
|
||
# Initialize the queue representation | ||
def _init(self, maxsize): | ||
self.queue = deque() | ||
|
||
def _qsize(self): | ||
return len(self.queue) | ||
|
||
# Put a new item in the queue | ||
def _put(self, item): | ||
self.queue.append(item) | ||
|
||
# Get an item from the queue | ||
def _get(self): | ||
return self.queue.popleft() |
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.
We use a bounded queue, but do we invoke it with block or timeout? I think this might be important because of how the
self.not_full.wait()
calls end up behaving with anRLock
.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.
we only use non-blocking put