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.

Sign up for GitHub

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

Jump to bottom

Fix #508: Py37 Deadlock ThreadPoolExecutor #598

Merged
merged 1 commit into from
May 15, 2020
Merged

Fix #508: Py37 Deadlock ThreadPoolExecutor #598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions eventlet/patcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,12 @@ def monkey_patch(**on):
import threading
threading.RLock = threading._PyRLock

# Issue #508: Since Python 3.7 queue.SimpleQueue is implemented in C,
# causing a deadlock. Replace the C implementation with the Python one.
if sys.version_info >= (3, 7):
import queue
queue.SimpleQueue = queue._PySimpleQueue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only do this when on['thread'] is true? It shouldn't be a problem otherwise, right?

OTOH, we have precedent for a blanket use-more-python approach up just a few lines above...



def is_monkey_patched(module):
"""Returns True if the given module is monkeypatched currently, False if
Expand Down
18 changes: 18 additions & 0 deletions tests/isolated/patcher_threadpoolexecutor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Issue #508: test ThreadPoolExecutor with monkey-patching
__test__ = False

if __name__ == '__main__':
import eventlet
eventlet.monkey_patch()

import sys

# Futures is only included in 3.2 or later
if sys.version_info >= (3, 2):
from concurrent import futures

with futures.ThreadPoolExecutor(max_workers=1) as executor:
future = executor.submit(pow, 2, 3)
res = future.result()
assert res == 8, '2^3 should be 8, not %s' % res
print('pass')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a bit of an indirect test; something like

import eventlet
eventlet.monkey_patch()
import threading
try:
    import queue
except ImportError:
    import Queue as queue  # py2

def tgt(q):
    print(q.get())

# SimpleQueue is sufficient, but was only added in 3.7
q = getattr(queue, 'SimpleQueue', queue.Queue)()
t = threading.Thread(target=tgt, args=(q,))
t.start()
q.put('pass')
t.join()

seems a little more obvious -- could even tighten it to just use eventlet.monkey_patch(all=False, thread=True).

OTOH, this more-clearly demonstrates the fact that we had a thing that was working when monkey-patched prior to 3.7 and stopped working after. *shrug*

4 changes: 4 additions & 0 deletions tests/patcher_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,3 +511,7 @@ def test_regular_file_readall():

def test_threading_current():
tests.run_isolated('patcher_threading_current.py')


def test_threadpoolexecutor():
tests.run_isolated('patcher_threadpoolexecutor.py')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a little worried about how tests still pass when I reverted the change to patcher.py ... but that's a problem in the test infrastructure, not this patch. Submitted #612 to address.