Tweak the use of minimal_ident to try to alleviate interpreter shutdo…

…wn crashes.

.travis.yml

@@ -8,7 +8,7 @@ python:
 env:
     global:
         - BUILD_RUNTIMES=$HOME/.runtimes
-        - PYTHONHASHSEED=random
+        - PYTHONHASHSEED=8675309
         - CC="ccache gcc"
         - CCACHE_NOCPP2=true
         - CCACHE_SLOPPINESS=file_macro,time_macros,include_file_ctime,include_file_mtime

appveyor.yml

@@ -5,6 +5,8 @@ environment:
     # /E:ON and /V:ON options are not enabled in the batch script interpreter
     # See: http://stackoverflow.com/a/13751649/163740
     CMD_IN_ENV: "cmd /E:ON /V:ON /C .\\appveyor\\run_with_env.cmd"
+    # Use a fixed hash seed for reproducability
+    PYTHONHASHSEED: 8675309
 
   matrix:
 

src/gevent/greenlet.py

@@ -302,11 +302,16 @@ def name(self):
         """
         The greenlet name. By default, a unique name is constructed using
         the :attr:`minimal_ident`. You can assign a string to this
-        value to change it. It is shown in the `repr` of this object.
+        value to change it. It is shown in the `repr` of this object if it
+        has been assigned to or if the `minimal_ident` has already been generated.
 
         .. versionadded:: 1.3a2
+        .. versionchanged:: 1.4
+           Stop showing generated names in the `repr` when the ``minimal_ident``
+           hasn't been requested. This reduces overhead and may be less confusing,
+           since ``minimal_ident`` can get reused.
         """
-        return 'Greenlet-%d' % (self.minimal_ident)
+        return 'Greenlet-%d' % (self.minimal_ident,)
 
     def _raise_exception(self):
         reraise(*self.exc_info)
@@ -426,7 +431,14 @@ def successful(self):
 
     def __repr__(self):
         classname = self.__class__.__name__
-        result = '<%s "%s" at %s' % (classname, self.name, hex(id(self)))
+        # If no name has been assigned, don't generate one, including a minimal_ident,
+        # if not necessary. This reduces the use of weak references and associated
+        # overhead.
+        if 'name' not in self.__dict__ and self._ident is None:
+            name = ' '
+        else:
+            name = ' "%s" ' % (self.name,)
+        result = '<%s%sat %s' % (classname, name, hex(id(self)))
         formatted = self._formatinfo()
         if formatted:
             result += ': ' + formatted

src/gevent/hub.py

@@ -339,8 +339,6 @@ def reinit(hub=None):
     #sleep(0.00001)
 
 
-hub_ident_registry = IdentRegistry()
-
 class Hub(WaitOperationsGreenlet):
     """
     A greenlet that runs the event loop.
@@ -386,6 +384,15 @@ class Hub(WaitOperationsGreenlet):
     # because that conflicts with the slot we inherit from the
     # Cythonized-bases.
 
+    # This is the source for our 'minimal_ident' property. We don't use a
+    # IdentRegistry because we've seen some crashes having to do with
+    # clearing weak references on shutdown in Windows (see known_failures.py).
+    # This gives us slightly different semantics than a greenlet's minimal_ident
+    # (notably, there can be holes) but we never documented this object's minimal_ident,
+    # and there should be few enough hub's over the lifetime of a process so as not
+    # to matter much.
+    _hub_counter = 0
+
     def __init__(self, loop=None, default=None):
         WaitOperationsGreenlet.__init__(self, None, None)
         self.thread_ident = get_thread_ident()
@@ -408,7 +415,9 @@ def __init__(self, loop=None, default=None):
         self._resolver = None
         self._threadpool = None
         self.format_context = GEVENT_CONFIG.format_context
-        self.minimal_ident = hub_ident_registry.get_ident(self)
+
+        Hub._hub_counter += 1
+        self.minimal_ident = Hub._hub_counter
 
     @Lazy
     def ident_registry(self):

src/gevent/testing/patched_tests_setup.py

@@ -461,9 +461,11 @@ def get_switch_expected(fullname):
             # Inserting GCs doesn't fix it.
             'test_ssl.ThreadedTests.test_handshake_timeout',
 
-            # These sometimes raise LoopExit, for no apparent reason.
+            # These sometimes raise LoopExit, for no apparent reason,
+            # mostly but not exclusively on Python 2.
             'test_socket.BufferIOTest.testRecvFromIntoBytearray',
             'test_socket.BufferIOTest.testRecvFromIntoArray',
+            'test_socket.BufferIOTest.testRecvFromIntoEmptyBuffer',
         ]
 
         if PY3:

src/gevent/tests/known_failures.py

@@ -110,7 +110,73 @@
                 #     File "C:\Python37-x64\lib\weakref.py", line 356 in remove
 
                 # ! C:\Python37-x64\python.exe -u -mgevent.tests.test__greenness [code 3221225477] [took 1.3s]
-                'FLAKY test__greenness.py',
+                # We have also seen this for Python 3.6.6 Nov 13 2018:
+                # | C:\Python36-x64\python.exe -u -mgevent.tests.test__backdoor
+                #   ss.s.s
+                #   ----------------------------------------------------------------------
+                #   Ran 6 tests in 0.953s
+
+                #   OK (skipped=4)
+                #   Windows fatal exception: access violation
+
+                #   Thread 0x00000aec (most recent call first):
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 84 in wait
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 166 in get
+                #     File "C:\Python36-x64\lib\site-packages\gevent\threadpool.py", line 270 in _worker
+
+                #   Thread 0x00000548 (most recent call first):
+
+                #   Thread 0x000003d0 (most recent call first):
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 84 in wait
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 166 in get
+                #     File "C:\Python36-x64\lib\site-packages\gevent\threadpool.py", line 270 in _worker
+
+                #   Thread 0x00000ad0 (most recent call first):
+
+                #   Thread 0x00000588 (most recent call first):
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 84 in wait
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 166 in get
+                #     File "C:\Python36-x64\lib\site-packages\gevent\threadpool.py", line 270 in _worker
+
+                #   Thread 0x00000a54 (most recent call first):
+
+                #   Thread 0x00000768 (most recent call first):
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 84 in wait
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 166 in get
+                #     File "C:\Python36-x64\lib\site-packages\gevent\threadpool.py", line 270 in _worker
+
+                #   Current thread 0x00000894 (most recent call first):
+                #     File "C:\Python36-x64\lib\site-packages\gevent\threadpool.py", line 261 in _worker
+
+                #   Thread 0x00000634 (most recent call first):
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 84 in wait
+                #     File "C:\Python36-x64\lib\site-packages\gevent\_threading.py", line 166 in get
+                #     File "C:\Python36-x64\lib\site-packages\gevent\threadpool.py", line 270 in _worker
+
+                #   Thread 0x00000538 (most recent call first):
+
+                #   Thread 0x0000049c (most recent call first):
+                #     File "C:\Python36-x64\lib\weakref.py", line 356 in remove
+
+                # ! C:\Python36-x64\python.exe -u -mgevent.tests.test__backdoor [code 3221225477] [Ran 6 tests in 2.1s]
+
+                # Note the common factors:
+                # - The test is finished (successfully) and we're apparently exiting the VM,
+                #   doing GC
+                # - A weakref is being cleaned up
+
+                # weakref.py line 356 remove() is in WeakKeyDictionary. We only use WeakKeyDictionary
+                # in gevent._ident.IdentRegistry, which is only used in two places:
+                # gevent.hub.hub_ident_registry, which has weak references to Hub objects,
+                # and gevent.greenlet.Greenlet.minimal_ident, which uses its parent Hub's
+                # IdentRegistry to get its own identifier. So basically they have weak references
+                # to Hub and arbitrary Greenlets.
+
+                # Our attempted solution: stop using a module-level IdentRegistry to get
+                # Hub idents, and reduce how often we auto-generate one for greenlets.
+                # Commenting out the tests, lets see if it works.
+                #'FLAKY test__greenness.py',
+                #'FLAKY test__backdoor.py',
             ]
 
     if not PY35:

src/gevent/tests/test__greenlet.py

@@ -424,13 +424,24 @@ def test_function(self):
         assert_not_ready(g)
         g.join()
         assert_ready(g)
-        self.assertTrue(hexobj.sub('X', str(g)).endswith(' at X: dummy_test_func>'))
+        self.assertTrue(hexobj.sub('X', str(g)).endswith(' at X: dummy_test_func>'), str(g))
 
     def test_method(self):
         g = gevent.Greenlet.spawn(A().method)
         str_g = hexobj.sub('X', str(g))
         str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.startswith('<Greenlet "Greenlet-'))
+        self.assertTrue(str_g.startswith('<Greenlet at X:'), str_g)
+        # Accessing the name to generate a minimal_ident will cause it to be included.
+        getattr(g, 'name')
+        str_g = hexobj.sub('X', str(g))
+        str_g = str_g.replace(__name__, 'module')
+        self.assertTrue(str_g.startswith('<Greenlet "Greenlet-'), str_g)
+        # Assigning to the name changes it
+        g.name = 'Foo'
+        str_g = hexobj.sub('X', str(g))
+        str_g = str_g.replace(__name__, 'module')
+        self.assertTrue(str_g.startswith('<Greenlet "Foo"'), str_g)
+
         self.assertTrue(str_g.endswith('at X: <bound method A.method of <module.A object at X>>>'))
         assert_not_ready(g)
         g.join()
@@ -443,7 +454,7 @@ def test_subclass(self):
         g = Subclass()
         str_g = hexobj.sub('X', str(g))
         str_g = str_g.replace(__name__, 'module')
-        self.assertTrue(str_g.startswith('<Subclass '))
+        self.assertTrue(str_g.startswith('<Subclass '), str_g)
         self.assertTrue(str_g.endswith('at X: _run>'))
 
         g = Subclass(None, 'question', answer=42)

src/gevent/tests/test__hub.py

@@ -115,8 +115,11 @@ def test(self):
 
         waiter = Waiter()
         g = gevent.spawn(waiter.get)
+        g.name = 'AName'
         gevent.sleep(0)
-        self.assertTrue(str(waiter).startswith('<Waiter greenlet=<Greenlet "Greenlet-'))
+        str_waiter = str(waiter)
+        self.assertTrue(str_waiter.startswith('<Waiter greenlet=<Greenlet "AName'),
+                        str_waiter)
 
         g.kill()