Merge pull request #1662 from gevent/issue1659

Fix issue 1659

.travis.yml

@@ -180,7 +180,11 @@ jobs:
         # First, the build dependencies (see setup.cfg)
         # so that we don't have to use build isolation and can better use the cache;
         # Note that we can't use -U for cffi and greenlet on PyPy.
-        - &build-gevent-deps pip install -U setuptools wheel twine && pip install -U 'faulthandler; python_version == "2.7" and platform_python_implementation == "CPython"' 'cffi;platform_python_implementation=="CPython"' 'cython>=3.0a5' 'greenlet;platform_python_implementation=="CPython"'
+        # The -q is because PyPy2 sometimes started raising
+        #   UnicodeEncodeError: 'ascii' codec can't encode character u'\u2588' in position 6: ordinal not in range(128)
+        # when downloading files. This started sometime in mid 2020. It's from
+        # pip's vendored progress.bar class.
+        - &build-gevent-deps pip install -U -q setuptools wheel twine && pip install -q -U 'faulthandler; python_version == "2.7" and platform_python_implementation == "CPython"' 'cffi;platform_python_implementation=="CPython"' 'cython>=3.0a5' 'greenlet;platform_python_implementation=="CPython"'
         # Next, build the wheel *in place*. This helps ccache, and also lets us cache the configure
         # output (pip install uses a random temporary directory, making this difficult)
         - python setup.py bdist_wheel

docs/changes/1659.bugfix

+The ``DummyThread`` objects created automatically by certain
+operations when the standard library threading module is
+monkey-patched now match the naming convention the standard library
+uses ("Dummy-12345"). Previously (since gevent 1.2a2) they used
+"DummyThread-12345".

docs/changes/1661.bugfix

+Fix compatibility with dnspython 2.
+
+.. caution:: This currently means that it can be imported. But it
+             cannot yet be used. gevent has a pinned dependency on
+             dnspython < 2 for now.

setup.py

@@ -291,7 +291,7 @@ del _to_cythonize
 ## Extras
 
 EXTRA_DNSPYTHON = [
-    'dnspython >= 1.16.0',
+    'dnspython >= 1.16.0, < 2.0',
     'idna',
 ]
 EXTRA_EVENTS = [

src/gevent/_sslgte279.py

@@ -74,6 +74,9 @@ class _fileobject(getattr(__ssl__, '_fileobject', object)): # pylint:disable=no-
 orig_SSLContext = __ssl__.SSLContext # pylint: disable=no-member
 
 class SSLContext(orig_SSLContext):
+
+    __slots__ = ()
+
     def wrap_socket(self, sock, server_side=False,
                     do_handshake_on_connect=True,
                     suppress_ragged_eofs=True,

src/gevent/_util.py

@@ -3,9 +3,10 @@
 internal gevent utilities, not for external use.
 """
 
-from __future__ import print_function, absolute_import, division
+# Be very careful not to import anything that would cause issues with
+# monkey-patching.
 
-from functools import update_wrapper
+from __future__ import print_function, absolute_import, division
 
 from gevent._compat import iteritems
 
@@ -23,6 +24,38 @@ class _NONE(object):
 
 _NONE = _NONE()
 
+WRAPPER_ASSIGNMENTS = ('__module__', '__name__', '__qualname__', '__doc__',
+                       '__annotations__')
+WRAPPER_UPDATES = ('__dict__',)
+def update_wrapper(wrapper,
+                   wrapped,
+                   assigned=WRAPPER_ASSIGNMENTS,
+                   updated=WRAPPER_UPDATES):
+    """
+    Based on code from the standard library ``functools``, but
+    doesn't perform any of the troublesome imports.
+
+    functools imports RLock from _thread for purposes of the
+    ``lru_cache``, making it problematic to use from gevent.
+
+    The other imports are somewhat heavy: abc, collections, types.
+    """
+    for attr in assigned:
+        try:
+            value = getattr(wrapped, attr)
+        except AttributeError:
+            pass
+        else:
+            setattr(wrapper, attr, value)
+    for attr in updated:
+        getattr(wrapper, attr).update(getattr(wrapped, attr, {}))
+    # Issue #17482: set __wrapped__ last so we don't inadvertently copy it
+    # from the wrapped function when updating __dict__
+    wrapper.__wrapped__ = wrapped
+    # Return the wrapper so this can be used as a decorator via partial()
+    return wrapper
+
+
 def copy_globals(source,
                  globs,
                  only_names=None,

src/gevent/resolver/dnspython.py

@@ -112,7 +112,7 @@ def _patch_dns():
     def extra_all(mod_name):
         return extras.get(mod_name, ())
 
-    def after_import_hook(mod):
+    def after_import_hook(dns): # pylint:disable=redefined-outer-name
         # Runs while still in the original patching scope.
         # The dns.rdata:get_rdata_class() function tries to
         # dynamically import modules using __import__ and then walk
@@ -122,10 +122,23 @@ def _patch_dns():
         # We could patch __import__ to do things at runtime, but it's
         # easier to enumerate the world and populate the cache now
         # before we then switch the names back.
-        rdata = mod.rdata
+        rdata = dns.rdata
         get_rdata_class = rdata.get_rdata_class
-        for rdclass in mod.rdataclass._by_value:
-            for rdtype in mod.rdatatype._by_value:
+        try:
+            rdclass_values = list(dns.rdataclass.RdataClass)
+        except AttributeError:
+            # dnspython < 2.0
+            rdclass_values = dns.rdataclass._by_value
+
+        try:
+            rdtype_values = list(dns.rdatatype.RdataType)
+        except AttributeError:
+            # dnspython < 2.0
+            rdtype_values = dns.rdatatype._by_value
+
+
+        for rdclass in rdclass_values:
+            for rdtype in rdtype_values:
                 get_rdata_class(rdclass, rdtype)
 
     patcher = importer('dns', extra_all, after_import_hook)
@@ -137,7 +150,6 @@ def _patch_dns():
 
     top.rdata.__import__ = _no_dynamic_imports
 
-
     return top
 
 dns = _patch_dns()

src/gevent/testing/testcase.py

@@ -432,3 +432,6 @@ class TestCase(TestCaseMetaClass("NewBase",
 
     assertRaisesRegex = getattr(BaseTestCase, 'assertRaisesRegex',
                                 getattr(BaseTestCase, 'assertRaisesRegexp'))
+
+    def assertStartsWith(self, it, has_prefix):
+        self.assertTrue(it.startswith(has_prefix), (it, has_prefix))

src/gevent/tests/test__monkey_logging.py

@@ -13,6 +13,21 @@ def _inner_lock(lock):
     attr = getattr(lock, '_block' if not PY2 else '_RLock__block', None)
     return attr
 
+def _check_type(root, lock, inner_semaphore, kind):
+    if not isinstance(inner_semaphore, kind):
+        raise AssertionError(
+            "Expected <object>.[_]lock._block to be of type %s, "
+            "but it was of type %s.\n"
+            "\t<object>.[_]lock=%r\n"
+            "\t<object>.[_]lock._block=%r\n"
+            "\t<object>=%r" % (
+                kind,
+                type(inner_semaphore),
+                lock,
+                inner_semaphore,
+                root
+            )
+        )
 
 def checkLocks(kind, ignore_none=True):
     handlers = logging._handlerList
@@ -21,15 +36,15 @@ def checkLocks(kind, ignore_none=True):
     for weakref in handlers:
         # In py26, these are actual handlers, not weakrefs
         handler = weakref() if callable(weakref) else weakref
-        attr = _inner_lock(handler.lock)
-        if attr is None and ignore_none:
+        block = _inner_lock(handler.lock)
+        if block is None and ignore_none:
             continue
-        assert isinstance(attr, kind), (handler.lock, attr, kind)
+        _check_type(handler, handler.lock, block, kind)
 
     attr = _inner_lock(logging._lock)
     if attr is None and ignore_none:
         return
-    assert isinstance(attr, kind)
+    _check_type(logging, logging._lock, attr, kind)
 
 checkLocks(type(threading._allocate_lock()))
 

src/gevent/tests/test__socket_dns6.py

@@ -10,8 +10,9 @@ from gevent.tests.test__socket_dns import TestCase, add
 
 from gevent.testing.sysinfo import OSX
 from gevent.testing.sysinfo import RESOLVER_DNSPYTHON
+from gevent.testing.sysinfo import RESOLVER_ARES
 from gevent.testing.sysinfo import PYPY
-
+from gevent.testing.sysinfo import PY2
 
 # We can't control the DNS servers on CI (or in general...)
 # for the system. This works best with the google DNS servers
@@ -45,6 +46,18 @@ class Test6(TestCase):
         # of the system and ares. They don't match exactly.
         return ()
 
+    if RESOLVER_ARES and PY2:
+        def _normalize_result_getnameinfo(self, result):
+            # Beginning 2020-07-23,
+            # c-ares returns a scope id on the result:
+            #    ('2001:470:1:18::115%0', 'http')
+            # The standard library does not (on linux or os x).
+            # I've only seen '%0', so only remove that
+            ipaddr, service = result
+            if ipaddr.endswith('%0'):
+                ipaddr = ipaddr[:-2]
+            return (ipaddr, service)
+
     if not OSX and RESOLVER_DNSPYTHON:
         # It raises gaierror instead of socket.error,
         # which is not great and leads to failures.

src/gevent/tests/test__threading_no_monkey.py

+# -*- coding: utf-8 -*-
+"""
+Tests for ``gevent.threading`` that DO NOT monkey patch. This
+allows easy comparison with the standard module.
+
+"""
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+import threading
+
+from gevent import threading as gthreading
+from gevent import testing
+
+class TestDummyThread(testing.TestCase):
+
+    def test_name(self):
+        # Matches the stdlib.
+        # https://github.com/gevent/gevent/issues/1659
+        std_dummy = threading._DummyThread()
+        gvt_dummy = gthreading._DummyThread()
+        self.assertIsNot(type(std_dummy), type(gvt_dummy))
+
+        self.assertStartsWith(std_dummy.name, 'Dummy-')
+        self.assertStartsWith(gvt_dummy.name, 'Dummy-')
+
+
+if __name__ == '__main__':
+    testing.main()

src/gevent/threading.py

@@ -33,6 +33,11 @@ __implements__ = [
     '_get_ident',
     '_sleep',
     '_DummyThread',
+    # RLock cannot go here, even though we need to import it.
+    # If it goes here, it replaces the RLock from the native
+    # threading module, but we really just need it here when some
+    # things import this module.
+    #'RLock',
 ]
 
 
@@ -43,11 +48,14 @@ from gevent.thread import start_new_thread as _start_new_thread
 from gevent.thread import allocate_lock as _allocate_lock
 from gevent.thread import get_ident as _get_ident
 from gevent.hub import sleep as _sleep, getcurrent
+from gevent.lock import RLock
 
 from gevent._compat import PY3
 from gevent._compat import PYPY
+from gevent._util import LazyOnClass
 
-# Exports, prevent unused import warnings
+# Exports, prevent unused import warnings.
+# XXX: Why don't we use __all__?
 local = local
 start_new_thread = _start_new_thread
 allocate_lock = _allocate_lock
@@ -56,6 +64,7 @@ _sleep = _sleep
 getcurrent = getcurrent
 
 Lock = _allocate_lock
+RLock = RLock
 
 
 def _cleanup(g):
@@ -80,7 +89,7 @@ class _DummyThread(_DummyThread_):
 
     # These objects are constructed quite frequently in some cases, so
     # the optimization matters: for example, in gunicorn, which uses
-    # pywsgi.WSGIServer, every request is handled in a new greenlet,
+    # pywsgi.WSGIServer, most every request is handled in a new greenlet,
     # and every request uses a logging.Logger to write the access log,
     # and every call to a log method captures the current thread (by
     # default).
@@ -113,11 +122,11 @@ class _DummyThread(_DummyThread_):
     def __init__(self): # pylint:disable=super-init-not-called
         #_DummyThread_.__init__(self)
 
-        # It'd be nice to use a pattern like "greenlet-%d", but maybe somebody out
-        # there is checking thread names...
-        self._name = self._Thread__name = __threading__._newname("DummyThread-%d")
+        # It'd be nice to use a pattern like "greenlet-%d", but there are definitely
+        # third-party libraries checking thread names to detect DummyThread objects.
+        self._name = self._Thread__name = __threading__._newname("Dummy-%d")
         # All dummy threads in the same native thread share the same ident
-        # (that of the native thread)
+        # (that of the native thread), unless we're monkey-patched.
         self._set_ident()
 
         g = getcurrent()
@@ -131,10 +140,8 @@ class _DummyThread(_DummyThread_):
         else:
             # ... so for them we use weakrefs.
             # See https://github.com/gevent/gevent/issues/918
-            global _weakref
-            if _weakref is None:
-                _weakref = __import__('weakref')
-            ref = _weakref.ref(g, _make_cleanup_id(gid))
+            ref = self.__weakref_ref
+            ref = ref(g, _make_cleanup_id(gid)) # pylint:disable=too-many-function-args
             self.__raw_ref = ref
 
     def _Thread__stop(self):
@@ -145,6 +152,10 @@ class _DummyThread(_DummyThread_):
     def _wait_for_tstate_lock(self, *args, **kwargs): # pylint:disable=signature-differs
         pass
 
+    @LazyOnClass
+    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