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
Clean up threading book-keeping at fork when monkey-patched #611
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -312,6 +312,26 @@ def monkey_patch(**on): | |
for attr_name in deleted: | ||
if hasattr(orig_mod, attr_name): | ||
delattr(orig_mod, attr_name) | ||
|
||
if name == 'threading' and sys.version_info >= (3, 7): | ||
def fix_threading_active( | ||
_os=original('os'), | ||
_global_dict=original('threading').current_thread.__globals__, | ||
_patched=orig_mod | ||
): | ||
_prefork_active = [None] | ||
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. py2/3 is SO annoying, even tho this code is py3 only you can't use nonlocal because it's a SyntaxError on py2 🙄 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. py2/3 is SO annoying - even tho this code is py3 only you can't use nonlocal because it's a SyntaxError on py2 🙄 |
||
|
||
def before_fork(): | ||
_prefork_active[0] = _global_dict['_active'] | ||
_global_dict['_active'] = _patched._active | ||
|
||
def after_fork(): | ||
_global_dict['_active'] = _prefork_active[0] | ||
|
||
_os.register_at_fork( | ||
before=before_fork, | ||
after_in_parent=after_fork) | ||
fix_threading_active() | ||
finally: | ||
imp.release_lock() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
# Monkey patching interferes with threading in Python 3.7 | ||
# https://github.com/eventlet/eventlet/issues/592 | ||
__test__ = False | ||
|
||
|
||
def check(n, mod, tag): | ||
assert len(mod._active) == n, 'Expected {} {} threads, got {}'.format(n, tag, mod._active) | ||
|
||
|
||
if __name__ == '__main__': | ||
import eventlet | ||
import eventlet.patcher | ||
eventlet.monkey_patch() | ||
import os | ||
import sys | ||
import threading | ||
_threading = eventlet.patcher.original('threading') | ||
import eventlet.green.threading | ||
|
||
def target(): | ||
eventlet.sleep(0.1) | ||
|
||
threads = [ | ||
threading.Thread(target=target, name='patched'), | ||
_threading.Thread(target=target, name='original-1'), | ||
_threading.Thread(target=target, name='original-2'), | ||
eventlet.green.threading.Thread(target=target, name='green-1'), | ||
eventlet.green.threading.Thread(target=target, name='green-2'), | ||
eventlet.green.threading.Thread(target=target, name='green-3'), | ||
] | ||
for t in threads: | ||
t.start() | ||
|
||
check(2, threading, 'pre-fork patched') | ||
check(3, _threading, 'pre-fork original') | ||
check(4, eventlet.green.threading, 'pre-fork green') | ||
|
||
if os.fork() == 0: | ||
# Inside the child, we should only have a main thread, | ||
# but old pythons make it difficult to ensure | ||
if sys.version_info >= (3, 7): | ||
check(1, threading, 'child post-fork patched') | ||
check(1, _threading, 'child post-fork original') | ||
check(1, eventlet.green.threading, 'child post-fork green') | ||
sys.exit() | ||
else: | ||
os.wait() | ||
|
||
check(2, threading, 'post-fork patched') | ||
check(3, _threading, 'post-fork original') | ||
check(4, eventlet.green.threading, 'post-fork green') | ||
|
||
for t in threads: | ||
t.join() | ||
|
||
check(1, threading, 'post-join patched') | ||
check(1, _threading, 'post-join original') | ||
check(1, eventlet.green.threading, 'post-join green') | ||
print('pass') |
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.
Or maybe this should say
... and hasattr(_os, 'register_at_fork'):
?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.
i think yes, they may remove register_at_fork in future version; checking py version in test is the right move though