GeezerVison is showing github.com — queue.Queue() is not reentrant, so signals and GC can cause deadlocks · Issue #59181 · python/cpython
queue.Queue() is not reentrant, so signals and GC can cause deadlocks #59181Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2018-01-15.23:27:50.989>
created_at = <Date 2012-06-01.08:43:03.911>
labels = ['3.7', 'type-bug', 'library']
title = 'queue.Queue() is not reentrant, so signals and GC can cause deadlocks'
updated_at = <Date 2018-01-17.01:04:26.008>
user = 'https://bugs.python.org/JohanAR' bugs.python.org fields:
activity = <Date 2018-01-17.01:04:26.008>
actor = 'gregory.p.smith'
assignee = 'none'
closed = True
closed_date = <Date 2018-01-15.23:27:50.989>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2012-06-01.08:43:03.911>
creator = 'JohanAR'
dependencies = []
files = ['25786', '25787', '46806', '47092']
hgrepos = []
issue_num = 14976
keywords = ['patch']
message_count = 51.0
messages = ['162062', '162065', '162069', '162076', '211436', '275181', '275377', '275404', '275421', '275485', '291764', '300421', '300422', '300455', '300478', '300489', '300491', '300499', '300504', '300505', '300509', '300510', '300511', '300532', '300567', '300574', '300575', '300577', '300581', '300723', '300825', '301170', '301173', '301175', '301176', '301207', '301299', '301321', '301322', '301378', '301379', '301380', '301383', '301386', '301409', '310014', '310025', '310026', '310109', '310111', '310125']
nosy_count = 15.0
nosy_names = ['gvanrossum', 'tim.peters', 'rhettinger', 'gregory.p.smith', 'ncoghlan', 'pitrou', 'zzzeek', 'python-dev', 'sbt', 'serhiy.storchaka', 'JohanAR', 'yselivanov', 'itamarst', 'davin', 'Catalin Patulea']
pr_nums = ['3346']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue14976'
versions = ['Python 3.7'] ActivityJohanAR commented on Jun 1, 2012 MannequinAuthor More actions Python 2.7.3 (default, Apr 20 2012, 22:39:59)
[GCC 4.6.3] on linux2
If a signal handler calls Queue.PriorityQueue.put and a second signal is received while one is already being processed, neither of the calls to put will terminate.
Highly dependent on timing so it might be difficult to reproduce. JohanAR commented on Jun 1, 2012 MannequinAuthor More actions Start queue_deadlock.py in one terminal and note the PID it prints.
Compile queue_sendint.c in another terminal and execute it with previous PID as only argument.
If the bug is triggered, nothing will print in the python terminal window when you press Enter. To terminate the application you can press Ctrl-\ sbt commented on Jun 1, 2012 I don't think there is anything special about PriorityQueue.
There is a similar concerning the use of the Python implementation of RLock in signal handlers -- see http://bugs.python.org/issue13697.
Maybe the signal handler should temporarily mask or ignore SIGINT while it runs. JohanAR commented on Jun 1, 2012 MannequinAuthor More actions I did read some more on the subject and it seems like using locks with interrupts is in general a very difficult problem and not limited to Python.
Don't know if this is considered common knowledge among programmers or if it would be useful with at least a warning on the signal manual page. itamarst commented on Feb 17, 2014 This is not specifically a signal issue; it can happen with garbage collection as well if you have a Queue.put that runs in __del__ or a weakref callback function.
This can happen in real code. In my case, a thread that reads log messages from a Queue and writes them to disk. The thread putting log messages into the Queue can deadlock if GC happens to cause a log message to be written right after Queue.put() acquired the lock.
(see itamarst/crochet#25). itamarst changed the title [-]Queue.PriorityQueue() is not interrupt safe[/-] [+]queue.Queue() is not reentrant, so signals and GC can cause deadlocks[/+] on Feb 17, 2014zzzeek commented on Sep 8, 2016 SQLAlchemy suffered from this issue long ago as we use a Queue for connections, which can be collected via weakref callback and sent back to put(), which we observed can occur via gc. For many years (like since 2007 or so) we've packaged a complete copy of Queue.Queue with an RLock inside of it to work around it.
I'm just noticing this issue because I'm writing a new connection pool implementation that tries to deal with this issue a little differently (not using Queue.Queue though). rhettinger commented on Sep 9, 2016 Guido opposes having us going down the path of using an RLock and altering the queue module to cope with reentrancy.
In the case of mixing signals with threads, we all agree with Johan Aires Rastén that "using locks with interrupts is in general a very difficult problem and not limited to Python", nor is it even limited to the queue module.
I believe that the usual technique for mixing signals and threads is to have the signal handler do the minimal work necessary to record the event without interacting with the rest of the program. Later, another thread can act on the recorded signal but can do so with normal use of locks so that code invariants are not violated. This design makes reasoning about the program more tractable. python-dev commented on Sep 9, 2016 zzzeek commented on Sep 9, 2016 yep, that's what im doing in my approach. though longer term thing, I noticed it's very hard to find documentation on exactly when gc might run. E.g. would it ever run if I did something innocuous, like "self.thread_id = None" (probably not). Just wasn't easy to get that answer. python-dev commented on Sep 9, 2016 itamarst commented on Apr 16, 2017 This bug was closed on the basis that signals + threads don't interact well. Which is a good point.
Unfortunately this bug can happen in cases that have nothing to do with signals. If you look at the title and some of the comments it also happens as a result of garbage collection: in __del__ methods and weakref callbacks.
Specifically, garbage collection can happen on any bytecode boundary and cause reentrancy problems with Queue.
The attached file is an attempt to demonstrate this: it runs and runs until GC happens and then deadlocks for me (on Python 3.6). I.e. it prints GC! and after that no further output is printed and no CPU usage reported.
Please re-open this bug: if you don't want to fix signal case that's fine, but the GC case is still an issue. pitrou commented on Sep 5, 2017 I don't mind someone to reimplement a full-fledged Queue in C. As for me, I am currently implementing a SimpleQueue in C that's reentrant and has only the most basic functionality. gpshead commented on Sep 5, 2017 pitrou commented on Sep 5, 2017 Le 05/09/2017 à 23:21, Gregory P. Smith a écrit :
FYI - here is an appropriately licensed (WTFPL) very simple lock-free queue implemented in C
Looks like it uses busy-waiting inside its get() equivalent.
Also we would need a portable version of the atomic instructions used
there (e.g. __sync_bool_compare_and_swap) :-) gpshead commented on Sep 5, 2017 yeah, there are others such as https://www.liblfds.org/ that seem better from that standpoint (gcc, and microsoft compiler support - I'm sure clang is fine as well - anything gcc can do they can do). Ensuring they're supported across build target platforms (all the hardware technically has the ability) is the fun part if we're going to go that route. pitrou commented on Sep 5, 2017 One tangential note about a potential full-fledged Queue in C: the pure Python implementation is fair towards consumers, as it uses a threading.Condition which is itself fair. Achieving the same thing in C may significantly complicate the implementation. Raymond would have to decide whether it's an important property to keep.
The SimpleQueue class, being a separate API, is not tied by this constraint. rhettinger commented on Sep 5, 2017 To handle the logging.handlers.QueueHandler case, the SimpleQueue needs a put_nowait() method (see line 1959 of Lib/logging/handlers.py.
[Mike Bayer]
I noticed it's very hard to find documentation on exactly when
gc might run.
The answer I got from the other coredevs is that GC can trigger whenever a new GCable object is allocated (pretty much any container or any pure python object). rhettinger commented on Sep 6, 2017 Just for the record, Guido explained his aversion to RLocks. Roughly: 1) RLocks are slower and more complex 2) It is difficult to correctly write code that can survive reentrancy, so it is easy fool yourself into believing you've written correct code 3) Getting a deadlock with a regular lock is a better outcome than having the invariants subtly violated, 4) Deadlocks have a more clear-cut failure mode and are easier to debug.
Guido also explained that he favored some sort of minimal queue class even if it might not handle all possible gc/weakref/signal/del induced issues. Essentially, he believes there is some value in harm reduction by minimizing the likelihood of a failure. The will help sysadmins who already have a practice of doing monitoring and occasional restarts as a way of mitigating transient issues. pitrou commented on Jan 15, 2018 Hi all,
The PR has been ready for quite some time now. Raymond posted some review comments back in September, which I addressed by making the requested changes.
If someone wants to add their comments, now is the time. Otherwise I plan to merge the PR sometimes soon, so that it gets in before 3.7 beta. pitrou commented on Jan 15, 2018 pitrou commented on Jan 15, 2018 Ok, there has been enough reviews in the last four months :-) This is now merged. gpshead commented on Jan 16, 2018 Catalin has been examining code... switching concurrent.futures.thread to use SimpleQueue instead of Queue is probably a good idea as the queues in there get used from weakref callbacks. pitrou commented on Jan 16, 2018 Could you open a new issue for it? gpshead commented on Jan 17, 2018
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|