Skip to content

Commit

Permalink
Fix #2020; add tests for this.
Browse files Browse the repository at this point in the history
The most compatible way to fix #2020 seems to be just preventing assigning __class__ in the first place.
We still make sure there is a valid _main_thread that is-a _MainThread though.
  • Loading branch information
jamadden committed Feb 13, 2024
1 parent eb3a308 commit 44cf28b
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 21 deletions.
4 changes: 4 additions & 0 deletions docs/changes/2020.bugfix
@@ -1,6 +1,10 @@
Add support for Python patch releases 3.11.8 and 3.12.2, which changed
internal details of threading.

As a result of these changes, note that it is no longer possible to
change the ``__class__`` of a ``gevent.threading._DummyThread``
object on those versions.

Other updates for compatibility with the standard library include:

- Errors raised from ``subprocess.Popen`` may not have a filename set.
Expand Down
5 changes: 2 additions & 3 deletions src/gevent/ssl.py
Expand Up @@ -347,10 +347,9 @@ def __init__(self, sock=None, keyfile=None, certfile=None,
# non-blocking
raise ValueError("do_handshake_on_connect should not be specified for non-blocking sockets")
self.do_handshake()

except socket_error as x:
except OSError:
self.close()
raise x
raise

def _gevent_sock_class(self, family, type, proto, fileno):
return _contextawaresock(family, type, proto, fileno, _wref(self))
Expand Down
63 changes: 63 additions & 0 deletions src/gevent/tests/test__threading_fork_from_dummy.py
@@ -0,0 +1,63 @@
# -*- coding: utf-8 -*-
"""
Tests for `issue 2020 <https://github.com/gevent/gevent/issues/2020>`_:
3.11.8 and 3.12.2 try to swizzle ``__class__`` of the dummy thread
found when forking from inside a greenlet.
"""
from gevent import monkey; monkey.patch_all() # pragma: testrunner-no-monkey-combine

import unittest
import gevent.testing as greentest

import tempfile

@greentest.skipOnWindows("Uses os.fork")
class Test(greentest.TestCase):


def test_fork_from_dummy_thread(self):
import os
import threading
import contextlib

import gevent

def do_it(out):
# Be sure we've put the DummyThread in the threading
# maps
assert isinstance(threading.current_thread(), threading._DummyThread)

with open(out, 'wt', encoding='utf-8') as f:
with contextlib.redirect_stderr(f):
r = os.fork()
if r == 0:
# In the child.
# Our stderr redirect above will capture the
# "Exception ignored in _after_fork()" that the interpreter
# prints; actually printing the main_thread will result in
# raising an exception if our patch doesn't work.
main = threading.main_thread()
print(main, file=f)
print(type(main), file=f)
return r

with tempfile.NamedTemporaryFile('w+t', suffix='.gevent_threading.txt') as tf:
glet = gevent.spawn(do_it, tf.name)
glet.join()
pid = glet.get()
if pid == 0:
# Dump the child process quickly
os._exit(0)

os.waitpid(pid, 0)
tf.seek(0, 0)
contents = tf.read()


self.assertIn("<class 'threading._MainThread'>", contents)
self.assertNotIn("_DummyThread", contents)


if __name__ == '__main__':
unittest.main()
138 changes: 120 additions & 18 deletions src/gevent/threading.py
Expand Up @@ -27,7 +27,7 @@

__implements__ = [
'local',
'_start_new_thread',
'_start_new_thread', # Gone in 3.13; now start_joinable_thread
'_allocate_lock',
'Lock',
'_get_ident',
Expand All @@ -41,8 +41,12 @@
]


import threading as __threading__
import threading as __threading__ # imports os, sys, _thread, functools, time, itertools
_DummyThread_ = __threading__._DummyThread
_MainThread_ = __threading__._MainThread
import os
import sys

from gevent.local import local
from gevent.thread import start_new_thread as _start_new_thread
from gevent.thread import allocate_lock as _allocate_lock
Expand Down Expand Up @@ -76,14 +80,19 @@ def _(_r):

_weakref = None


class _DummyThread(_DummyThread_):
# We avoid calling the superclass constructor. This makes us about
# twice as fast (1.16 vs 0.68usec on PyPy, 29.3 vs 17.7usec on
# CPython 2.7), and has the important effect of avoiding
# allocation and then immediate deletion of _Thread__block, a
# lock. This is especially important on PyPy where locks go
# through the cpyext API and Cython, which is known to be slow and
# potentially buggy (e.g.,
# twice as fast:
#
# - 1.16 vs 0.68usec on PyPy (unknown version, older Intel mac)
# - 29.3 vs 17.7usec on CPython 2.7 (older intel Mac)
# - 0.98 vs 2.95usec on CPython 3.12.2 (newer M2 mac)
#
# It als has the important effect of avoiding allocation and then
# immediate deletion of _Thread__block, a lock. This is especially
# important on PyPy where locks go through the cpyext API and
# Cython, which is known to be slow and potentially buggy (e.g.,
# https://bitbucket.org/pypy/pypy/issues/2149/memory-leak-for-python-subclass-of-cpyext#comment-22347393)

# These objects are constructed quite frequently in some cases, so
Expand Down Expand Up @@ -117,6 +126,7 @@ class _DummyThread(_DummyThread_):
_Thread__started = _started = __threading__.Event()
_Thread__started.set()
_tstate_lock = None
_handle = None # 3.13

def __init__(self): # pylint:disable=super-init-not-called
#_DummyThread_.__init__(self)
Expand Down Expand Up @@ -156,16 +166,108 @@ def _wait_for_tstate_lock(self, *args, **kwargs): # pylint:disable=signature-dif
def __weakref_ref(self):
return __import__('weakref').ref

if hasattr(__threading__, 'main_thread'): # py 3.4+
def main_native_thread():
return __threading__.main_thread() # pylint:disable=no-member
else:
def main_native_thread():
main_threads = [v for v in __threading__._active.values()
if isinstance(v, __threading__._MainThread)]
assert len(main_threads) == 1, "Too many main threads"

return main_threads[0]
# In Python 3.11.8+ and 3.12.2+ (yes, minor patch releases),
# CPython's ``threading._after_fork`` hook began swizzling the
# type of the _DummyThread into _MainThread if such a dummy thread
# was the current thread when ``os.fork()`` gets called.
# From CPython's perspective, that's a more-or-less fine thing to do.
# While _DummyThread isn't a subclass of _MainThread, they are both
# subclasses of Thread, and _MainThread doesn't add any new instance
# variables.
#
# From gevent's perspective, that's NOT good. Our _DummyThread
# doesn't have all the instance variables that Thread does, and so
# attempting to do anything with this now-fake _MainThread doesn't work.
# You in fact immediately get assertion errors from inside ``_after_fork``.
# Now, these are basically harmless --- they're printed, and they prevent the cleanup
# of some globals in _threading, but that probably doesn't matter --- but
# people complained, and it could break some test scenarios (due to unexpected
# output on stderr, for example)
#
# We thought of a few options to patch around this:
#
# - Live with the performance penalty. Newer CPythons are making it
# harder and harder to perform well, so if we can possibly avoid
# adding our own performance regressions, that would be good.
#
# - ``after_fork`` uses ``isinstance(current, _DummyThread)``
# before swizzling, so we could use a metaclass to make that
# check return false. That's a fairly large compatibility risk,
# both because of the use of a metaclass (what if some other
# subclass of _DummyTHread is using an incompatible metaclass?)
# and the change in ``isinstance`` behaviour. We could limit the latter
# to a window around the fork, using ``os.register_at_fork(before, after_in_parent=)``,
# but that's a lot of moving pieces requiring the use of a global or class
# variable to track state.
#
# - We could copy the ivars of the current main thread into the
# _DummyThread in ``register_at_fork(before=)``. That appears to
# work, but also requires the use of
# ``register_at_fork(after_in_parent=)`` to reverse it.
#
# - We could simply prevent swizzling the class in the first
# place. In combination with
# ``register_at_fork(after_in_child=)`` to establish a *real*
# new _MainThread, that's a clean solution. Establishing a real
# new _MainThread is something that CPython itself is prepared
# to do if it can't figure out what the current thread is. The
# compatibility risk of this is relatively low: swizzling
# classes is frowned upon and uncommon, and we can limit it to
# just preventing this specific case. And if somebody was
# attempting this already with some other thread subclass, it
# would (probably?) have the exact same issues, so we can be pretty
# sure nobody is doing that.
#
# We're initially going with the last fix.
#
# Now, all of this is moot in 3.13, which takes a very different
# approach to handling this, and also changes some names. See
# https://github.com/python/cpython/commit/0e9c364f4ac18a2237bdbac702b96bcf8ef9cb09

# Tests pass just fine in 3.8 (and presumably 3.9 and 3.10) with these fixes
# applied, but just in case, we only do it where we know it's necessary.
_NEEDS_CLASS_FORK_FIXUP = (
(sys.version_info[:2] == (3, 11) and sys.version_info[:3] >= (3, 11, 8))
or sys.version_info[:3] >= (3, 12, 2)
)

# Override with a property, as opposed to using __setattr__,
# to avoid adding overhead on any other attribute setting.
@property
def __class__(self):
return type(self)

@__class__.setter
def __class__(self, new_class):
# Even if we wanted to allow setting this, I'm not sure
# exactly how to do so when we have a property object handling it.
# Getting the descriptor from ``object.__dict__['__class__']``
# and using its ``__set__`` method raises a TypeError (as does
# the simpler ``super().__class__``).
#
# Better allow the TypeError for now as opposed to silently ignoring
# the assignment.
if new_class is not _MainThread_:
object.__dict__['__class__'].__set__(self, new_class)

if _DummyThread._NEEDS_CLASS_FORK_FIXUP:

def _after_fork_in_child():
# We've already imported threading, which installed its "after" hook,
# so we're going to be called after that hook.
active = __threading__._active
assert len(active) == 1
main = __threading__._MainThread()
__threading__._active[__threading__.get_ident()] = main
__threading__._main_thread = main
assert main.ident == __threading__.get_ident()


if hasattr(os, 'register_at_fork'):
os.register_at_fork(after_in_child=_after_fork_in_child)

def main_native_thread():
return __threading__.main_thread() # pylint:disable=no-member


# XXX: Issue 18808 breaks us on Python 3.4+.
Expand Down

0 comments on commit 44cf28b

Please sign in to comment.