queue.Queue() is not reentrant, so signals and GC can cause deadlocks

[JohanAR]

BPO	14976
Nosy	@gvanrossum, @tim-one, @rhettinger, @gpshead, @ncoghlan, @pitrou, @serhiy-storchaka, @1st1, @applio
PRs	bpo-14976: Reentrant simple queue #3346
Files	queue_deadlock.py: signal hander reading/writing a queue queue_sendint.c: send 2 signals to specified process in rapid succession queuebug2.py: gc-based-queue-problem.py q_reentrancy.patch

^{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']

[JohanAR]

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]

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]

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]

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]

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).

[zzzeek]

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]

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]

New changeset 137806ca59ce by Gregory P. Smith in branch '3.5':
Add a note about queue not being safe for use from signal handlers.
https://hg.python.org/cpython/rev/137806ca59ce

New changeset 4e15e7618715 by Gregory P. Smith in branch 'default':
Add a note about queue not being safe for use from signal handlers.
https://hg.python.org/cpython/rev/4e15e7618715

[zzzeek]

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]

New changeset 8c00cbbd3ff9 by Raymond Hettinger in branch '3.5':
bpo-14976: Note that the queue module is not designed to protect against reentrancy
https://hg.python.org/cpython/rev/8c00cbbd3ff9

[itamarst]

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.