Merge pull request #1638 from gevent/issue1637

Fixes for Fedora Rawhide and SSL

docs/changes/1637.bugfix

+Python 3 ``gevent.ssl.SSLSocket`` objects no longer attempt to catch
+``ConnectionResetError`` and treat it the same as an ``SSLError`` with
+``SSL_ERROR_EOF`` (typically by suppressing it).
+
+This was a difference from the way the standard library behaved (which
+is to raise the exception). It was added to gevent during early
+testing of OpenSSL 1.1 and TLS 1.3.

scripts/releases/make-manylinux

@@ -44,6 +44,8 @@ if [ -d /gevent -a -d /opt/python ]; then
     export XDG_CACHE_HOME="/cache"
     ls -ld /cache
     yum -y install libffi-devel ccache
+    # On Fedora Rawhide (F33)
+    # yum install python39 python3-devel gcc kernel-devel kernel-headers make diffutils file
 
     mkdir /tmp/build
     cd /tmp/build

src/gevent/_ssl3.py

@@ -396,15 +396,15 @@ class SSLSocket(socket):
                 if ex.args[0] == SSL_ERROR_EOF and self.suppress_ragged_eofs:
                     return b'' if buffer is None else len(buffer) - initial_buf_len
                 raise
-            except ConnectionResetError:
-                # Certain versions of Python, built against certain
-                # versions of OpenSSL operating in certain modes,
-                # can produce this instead of SSLError. Notably, it looks
-                # like anything built against 1.1.1c do?
-                if self.suppress_ragged_eofs:
-                    return b'' if buffer is None else len(buffer) - initial_buf_len
-                raise
-
+            # Certain versions of Python, built against certain
+            # versions of OpenSSL operating in certain modes, can
+            # produce ``ConnectionResetError`` instead of
+            # ``SSLError``. Notably, it looks like anything built
+            # against 1.1.1c does that? gevent briefly (from support of TLS 1.3
+            # in Sept 2019 to issue #1637 it June 2020) caught that error and treaded
+            # it just like SSL_ERROR_EOF. But that's not what the standard library does.
+            # So presumably errors that result from unexpected ``ConnectionResetError``
+            # are issues in gevent tests.
 
     def write(self, data):
         """Write DATA to the underlying SSL channel.  Returns
@@ -612,6 +612,7 @@ class SSLSocket(socket):
             # that with a layer.
             shutdown = self._sslobj.unwrap
 
+        s = self._sock
         while True:
             try:
                 s = shutdown()
@@ -627,6 +628,12 @@ class SSLSocket(socket):
                 if self.timeout == 0.0:
                     raise
                 self._wait(self._write_event)
+            except OSError as e:
+                if e.errno == 0:
+                    # What does this even mean? Seen on 3.7+.
+                    break
+                raise
+
 
         self._sslobj = None
 

src/gevent/_sslgte279.py

@@ -19,7 +19,7 @@ _ssl = __ssl__._ssl # pylint:disable=no-member
 
 import errno
 from gevent._socket2 import socket
-from gevent._socket2 import AF_INET
+from gevent._socket2 import AF_INET # pylint:disable=no-name-in-module
 from gevent.socket import timeout_default
 from gevent.socket import create_connection
 from gevent.socket import error as socket_error
@@ -564,7 +564,13 @@ class SSLSocket(socket):
         if not self._sslobj:
             raise ValueError("No SSL wrapper around " + str(self))
 
-        s = self._sslobj_shutdown()
+        s = self._sock
+        try:
+            s = self._sslobj_shutdown()
+        except socket_error as ex:
+            if ex.args[0] != 0:
+                raise
+
         self._sslobj = None
         # match _ssl2; critical to drop/reuse here on PyPy
         # XXX: _ssl3 returns an SSLSocket. Is that what the standard lib does on

src/gevent/testing/openfiles.py

@@ -49,14 +49,14 @@ def _collects(func):
     # collected and drop references and thus close files. We should be deterministic
     # and careful about closing things.
     @functools.wraps(func)
-    def f():
+    def f(**kw):
         gc.collect()
         gc.collect()
         enabled = gc.isenabled()
         gc.disable()
 
         try:
-            return func()
+            return func(**kw)
         finally:
             if enabled:
                 gc.enable()
@@ -82,7 +82,7 @@ else:
         os.remove(tmpname)
         return data
 
-def default_get_open_files(pipes=False):
+def default_get_open_files(pipes=False, **_kwargs):
     data = _run_lsof()
     results = {}
     for line in data.split('\n'):
@@ -128,9 +128,13 @@ except ImportError:
     get_open_files = default_get_open_files
     get_number_open_files = default_get_number_open_files
 else:
+    class _TrivialOpenFile(object):
+        __slots__ = ('fd',)
+        def __init__(self, fd):
+            self.fd = fd
 
     @_collects
-    def get_open_files():
+    def get_open_files(count_closing_as_open=True, **_kw):
         """
         Return a list of popenfile and pconn objects.
 
@@ -146,8 +150,27 @@ else:
 
         for _ in range(3):
             try:
+                if count_closing_as_open and os.path.exists('/proc/'):
+                    # Linux only.
+                    # psutil doesn't always see all connections, even though
+                    # they exist in the filesystem. It's not entirely clear why.
+                    # It sees them on Travis (prior to Ubuntu Bionic, at least)
+                    # but doesn't in the manylinux image or Fedora 33 Rawhide image.
+                    # This happens in test__makefile_ref TestSSL.*; in particular, if a
+                    # ``sslsock.makefile()`` is opened and used to read all data, and the sending
+                    # side shuts down, psutil no longer finds the open file. So we add them
+                    # back in.
+                    #
+                    # Of course, the flip side of this is that we sometimes find connections
+                    # we're not expecting.
+                    # I *think* this has to do with CLOSE_WAIT handling?
+                    fd_directory = '/proc/%d/fd' % os.getpid()
+                    fd_files = os.listdir(fd_directory)
+                else:
+                    fd_files = []
                 process = psutil.Process()
-                results['data'] = process.open_files() + process.connections('all')
+                results['data'] = process.open_files()
+                results['data'] += process.connections('all')
                 break
             except OSError:
                 pass
@@ -157,7 +180,12 @@ else:
 
         for x in results['data']:
             results[x.fd] = x
-        results['data'] += ['From psutil', process]
+        for fd_str in fd_files:
+            if fd_str not in results:
+                fd = int(fd_str)
+                results[fd] = _TrivialOpenFile(fd)
+        results['data'] += [('From psutil', process)]
+        results['data'] += [('fd files', fd_files)]
         return results
 
     @_collects

src/gevent/testing/patched_tests_setup.py

@@ -1228,7 +1228,9 @@ if PY37:
 
     if APPVEYOR:
         disabled_tests += [
-
+            # This sometimes produces ``self.assertEqual(1, len(s.select(0))): 1 != 0``.
+            # Probably needs to spin the loop once.
+            'test_selectors.DefaultSelectorTestCase.test_timeout',
         ]
 
 if PY38:
@@ -1283,16 +1285,13 @@ if OSX:
     disabled_tests += [
         # This sometimes produces OSError: Errno 40: Message too long
         'test_socket.RecvmsgIntoTCPTest.testRecvmsgIntoGenerator',
-    ]
-
-    if RUNNING_ON_CI:
 
-        disabled_tests += [
-            # These sometime timeout. Cannot reproduce locally.
-            'test_ftp.TestTLS_FTPClassMixin.test_mlsd',
-            'test_ftp.TestTLS_FTPClassMixin.test_retrlines_too_long',
-            'test_ftp.TestTLS_FTPClassMixin.test_storlines',
-        ]
+        # These sometime timeout. Cannot reproduce locally.
+        'test_ftp.TestTLS_FTPClassMixin.test_mlsd',
+        'test_ftp.TestTLS_FTPClassMixin.test_retrlines_too_long',
+        'test_ftp.TestTLS_FTPClassMixin.test_storlines',
+        'test_ftp.TestTLS_FTPClassMixin.test_retrbinary_rest',
+    ]
 
     if RESOLVER_ARES and PY38 and not RUNNING_ON_CI:
         disabled_tests += [

src/gevent/tests/test__close_backend_fd.py

@@ -32,6 +32,13 @@ class Test(unittest.TestCase):
     )
 
     BACKENDS_THAT_WILL_FAIL_TO_CREATE_AT_RUNTIME = (
+        # This fails on the Ubuntu Bionic image, and on
+        # the Fedora Rawhide 33 image. It's not clear why; needs
+        # investigated.
+        'linux_iouring',
+    )
+
+    BACKENDS_THAT_WILL_FAIL_TO_CREATE_AT_RUNTIME += (
         # This can be compiled on any (?) version of
         # linux, but there's a runtime check that you're
         # running at least kernel 4.19, so we can fail to create

src/gevent/tests/test__makefile_ref.py

@@ -12,7 +12,6 @@ import gevent.testing as greentest
 from gevent.testing.params import DEFAULT_BIND_ADDR_TUPLE
 from gevent.testing.params import DEFAULT_CONNECT
 from gevent.testing.sockets import tcp_listener
-from gevent.testing.skipping import skipOnManylinux
 
 dirname = os.path.dirname(os.path.abspath(__file__))
 certfile = os.path.join(dirname, '2_7_keycert.pem')
@@ -75,7 +74,9 @@ class Test(greentest.TestCase):
     def assert_fd_closed(self, fileno):
         assert isinstance(fileno, fd_types), repr(fileno)
         assert fileno > 0, fileno
-        open_files = get_open_files()
+        # Here, if we're in the process of closing, don't consider it open.
+        # This goes into details of psutil
+        open_files = get_open_files(count_closing_as_open=False)
         if fileno in open_files:
             raise AssertionError('%r is not closed:\n%s' % (fileno, open_files['data']))
 
@@ -248,7 +249,6 @@ class TestSocket(Test):
 
 
 @greentest.skipOnAppVeyor("This sometimes times out for no apparent reason.")
-@skipOnManylinux("For some reason manylinux doesn't see the open files all the time.")
 class TestSSL(Test):
 
     def _ssl_connect_task(self, connector, port, accepted_event):
@@ -409,7 +409,6 @@ class TestSSL(Test):
             f.close()
             self.assert_closed(client_socket, fileno)
 
-    @skipOnManylinux("Doesn't see the file open")
     def test_serverssl_makefile2(self):
         raw_listener = tcp_listener(backlog=1)
         port = raw_listener.getsockname()[1]
@@ -439,16 +438,17 @@ class TestSSL(Test):
             self.assert_open(client_socket, fileno)
             self.assertEqual(f.read(), 'test_serverssl_makefile2')
             self.assertEqual(f.read(), '')
+            # Closing file object does not close the socket.
             f.close()
             if WIN and psutil:
                 # Hmm?
                 self.extra_allowed_open_states = (psutil.CONN_CLOSE_WAIT,)
+
             self.assert_open(client_socket, fileno)
             client_socket.close()
             self.assert_closed(client_socket, fileno)
 
 
-
 class Closing(object):
 
     def __init__(self, *init):

src/gevent/tests/test__server.py

@@ -309,6 +309,7 @@ class TestDefaultSpawn(TestCase):
             self.ServerClass(self.get_listener(), backlog=25)
 
     @greentest.skipOnLibuvOnCIOnPyPy("Sometimes times out")
+    @greentest.skipOnAppVeyor("Sometimes times out.")
     def test_backlog_is_accepted_for_address(self):
         self.server = self.ServerSubClass((greentest.DEFAULT_BIND_ADDR, 0), backlog=25)
         self.assertConnectionRefused()

src/gevent/tests/test__socket.py

@@ -3,7 +3,8 @@
 from __future__ import print_function
 from __future__ import absolute_import
 
-from gevent import monkey; monkey.patch_all()
+from gevent import monkey
+monkey.patch_all()
 
 import sys
 import array
@@ -149,11 +150,11 @@ class TestTCP(greentest.TestCase):
             conn, _ = self.listener.accept()
             try:
                 with conn.makefile(mode='rb') as r:
-                    log("\taccepted on server", conn)
+                    log("\taccepted on server; client conn is", conn, "file is", r)
                     accepted_event.set()
                     log("\treading")
                     read_data.append(r.read())
-                    log("\tdone reading")
+                    log("\tdone reading", r, "got bytes", len(read_data[0]))
                 del r
             finally:
                 conn.close()
@@ -165,17 +166,6 @@ class TestTCP(greentest.TestCase):
             log("creating client connection")
             client = self.create_connection(**client_args)
 
-            # We seem to have a buffer stuck somewhere on appveyor?
-            # https://ci.appveyor.com/project/denik/gevent/builds/27320824/job/bdbax88sqnjoti6i#L712
-            should_unwrap = hasattr(client, 'unwrap') and greentest.PY37 and greentest.WIN
-
-            # The implicit reference-based nastiness of Python 2
-            # sockets interferes, especially when using SSL sockets.
-            # The best way to get a decent FIN to the server is to shutdown
-            # the output. Doing that on Python 3, OTOH, is contraindicated
-            # except on PyPy.
-            should_shutdown = greentest.PY2 or greentest.PYPY
-
             # It's important to wait for the server to fully accept before
             # we shutdown and close the socket. In SSL mode, the number
             # and timing of data exchanges to complete the handshake and
@@ -192,33 +182,37 @@ class TestTCP(greentest.TestCase):
             # when it got switched to by server.join(), found its new socket
             # dead.
             accepted_event.wait()
-            log("accepted", client)
+            log("Client got accepted event from server", client, "; sending data", len(data))
             try:
-                getattr(client, client_method)(data)
-            except:
-                # unwrapping might not work after this because we're in
-                # a bad state.
-                if should_unwrap:
-                    client.shutdown(socket.SHUT_RDWR)
-                    should_unwrap = False
-                    should_shutdown = False
-                raise
+                x = getattr(client, client_method)(data)
+                log("Client sent data: result from method", x)
             finally:
-                log("shutdown")
-                if should_shutdown:
-                    client.shutdown(socket.SHUT_RDWR)
-                elif should_unwrap:
+                log("Client will unwrap and shutdown")
+                if hasattr(client, 'unwrap'):
+                    # Are we dealing with an SSLSocket? If so, unwrap it
+                    # before attempting to shut down the socket. This does the
+                    # SSL shutdown handshake and (hopefully) stops ``accept_and_read``
+                    # from generating ``ConnectionResetError`` on AppVeyor.
                     try:
-                        client.unwrap()
-                    except OSError as e:
-                        if greentest.PY37 and greentest.WIN and e.errno == 0:
-                            # ? 3.7.4 on AppVeyor sometimes raises
-                            # "OSError[errno 0] Error" here, which doesn't make
-                            # any sense.
-                            pass
-                        else:
-                            raise
-                log("closing")
+                        client = client.unwrap()
+                    except ValueError:
+                        pass
+
+                try:
+                    # The implicit reference-based nastiness of Python 2
+                    # sockets interferes, especially when using SSL sockets.
+                    # The best way to get a decent FIN to the server is to shutdown
+                    # the output. Doing that on Python 3, OTOH, is contraindicated
+                    # except on PyPy, so this used to read ``PY2 or PYPY``. But
+                    # it seems that a shutdown is generally good practice, and I didn't
+                    # document what errors we saw without it. Per issue #1637
+                    # lets do a shutdown everywhere, but only after removing any
+                    # SSL wrapping.
+                    client.shutdown(socket.SHUT_RDWR)
+                except (OSError, socket.error):
+                    pass
+
+                log("Client will close")
                 client.close()
         finally:
             server.join(10)
@@ -231,6 +225,8 @@ class TestTCP(greentest.TestCase):
             match_data = self.long_data
         read_data = read_data[0].split(b',')
         match_data = match_data.split(b',')
+        self.assertEqual(read_data[0], match_data[0])
+        self.assertEqual(len(read_data), len(match_data))
         self.assertEqual(read_data, match_data)
 
     def test_sendall_str(self):

src/gevent/tests/test__ssl.py

 from __future__ import print_function, division, absolute_import
-from gevent import monkey; monkey.patch_all()
+from gevent import monkey
+monkey.patch_all()
 import os
 
 import socket

src/gevent/tests/test__threading_2.py

@@ -394,12 +394,12 @@ class ThreadTests(unittest.TestCase):
             warnings.simplefilter('ignore', DeprecationWarning)
             # get/set checkinterval are deprecated in Python 3,
             # and removed in Python 3.9
-            old_interval = sys.getcheckinterval()
+            old_interval = sys.getcheckinterval() # pylint:disable=no-member
             try:
                 for i in xrange(1, 100):
                     # Try a couple times at each thread-switching interval
                     # to get more interleavings.
-                    sys.setcheckinterval(i // 5)
+                    sys.setcheckinterval(i // 5) # pylint:disable=no-member
                     t = threading.Thread(target=lambda: None)
                     t.start()
                     t.join()
@@ -407,7 +407,7 @@ class ThreadTests(unittest.TestCase):
                     self.assertFalse(t in l,
                                      "#1703448 triggered after %d trials: %s" % (i, l))
             finally:
-                sys.setcheckinterval(old_interval)
+                sys.setcheckinterval(old_interval) # pylint:disable=no-member
 
     if not hasattr(sys, 'pypy_version_info'):
         def test_no_refcycle_through_target(self):