Merge pull request #1055 from gevent/fix_test__socket

Attempt to fix (or workaround) test__socket.py on Windows

src/greentest/greentest.py

@@ -97,6 +97,9 @@ if WIN:
     NON_APPLICABLE_SUFFIXES.append("posix")
     # This is intimately tied to FileObjectPosix
     NON_APPLICABLE_SUFFIXES.append("fileobject2")
+    SHARED_OBJECT_EXTENSION = ".pyd"
+else:
+    SHARED_OBJECT_EXTENSION = ".so"
 
 
 RUNNING_ON_TRAVIS = os.environ.get('TRAVIS')
@@ -251,8 +254,10 @@ def wrap_refcount(method):
                 d = sum(hist_before.values())
 
                 self.setUp()
-                method(self, *args, **kwargs)
-                self.tearDown()
+                try:
+                    method(self, *args, **kwargs)
+                finally:
+                    self.tearDown()
 
                 # Grab post snapshot
                 if 'urlparse' in sys.modules:
@@ -419,6 +424,10 @@ class TestCase(TestCaseMetaClass("NewBase", (BaseTestCase,), {})):
             self.switch_expected = get_switch_expected(self.fullname)
         return BaseTestCase.run(self, *args, **kwargs)
 
+    def setUp(self):
+        super(TestCase, self).setUp()
+        self.close_on_teardown = []
+
     def tearDown(self):
         if getattr(self, 'skipTearDown', False):
             return
@@ -426,10 +435,7 @@ class TestCase(TestCaseMetaClass("NewBase", (BaseTestCase,), {})):
             self.cleanup()
         self._error = self._none
         self._tearDownCloseOnTearDown()
-        try:
-            del self.close_on_teardown
-        except AttributeError:
-            pass
+        self.close_on_teardown = []
         super(TestCase, self).tearDown()
 
     def _tearDownCloseOnTearDown(self):
@@ -441,7 +447,6 @@ class TestCase(TestCaseMetaClass("NewBase", (BaseTestCase,), {})):
             except Exception:
                 pass
 
-
     @classmethod
     def setUpClass(cls):
         import warnings
@@ -461,8 +466,6 @@ class TestCase(TestCaseMetaClass("NewBase", (BaseTestCase,), {})):
         *resource* either has a ``close`` method, or is a
         callable.
         """
-        if 'close_on_teardown' not in self.__dict__:
-            self.close_on_teardown = []
         self.close_on_teardown.append(resource)
         return resource
 
@@ -762,7 +765,7 @@ def walk_modules(basedir=None, modpath=None, include_so=False, recursive=False):
                 except ImportError:
                     continue
             yield path, modpath + x
-        elif include_so and fn.endswith('.so'):
+        elif include_so and fn.endswith(SHARED_OBJECT_EXTENSION):
             if '.pypy-' in fn:
                 continue
             if fn.endswith('_d.so'):

src/greentest/known_failures.py

@@ -72,15 +72,6 @@ if sys.platform == 'win32':
             'FLAKY test__fileobject.py',
         ]
 
-        FAILING_TESTS += [
-            # This sometimes fails with a timeout, meaning
-            # one of the tests hangs (test_fullduplex, maybe?).
-            # But only sometimes, and only seen on Py2.7, beginning
-            # ~2016-02-24.
-            # Beginning Apr 2016 sometimes also seen with Py 3.5
-            'FLAKY test__socket.py',
-        ]
-
         if PY3:
             FAILING_TESTS += [
                 # test_set_and_clear in Py3 relies on 5 threads all starting and

src/greentest/test__makefile_ref.py

@@ -119,15 +119,12 @@ class Test(greentest.TestCase):
         # Keeping raw sockets alive keeps SSL sockets
         # from being closed too, at least on CPython, so we
         # need to use weakrefs
-        if 'close_on_teardown' not in self.__dict__:
-            self.close_on_teardown = []
         self.close_on_teardown.append(weakref.ref(resource))
         return resource
 
     def _tearDownCloseOnTearDown(self):
         self.close_on_teardown = [r() for r in self.close_on_teardown if r() is not None]
         super(Test, self)._tearDownCloseOnTearDown()
-        self.close_on_teardown = []
 
 class TestSocket(Test):
 

src/greentest/test__socket.py

@@ -46,19 +46,30 @@ class TestTCP(greentest.TestCase):
         long_data = long_data.encode('ascii')
 
     def setUp(self):
-        greentest.TestCase.setUp(self)
+        super(TestTCP, self).setUp()
+        self.listener = self._close_on_teardown(self._setup_listener())
+
+        # XXX: On Windows (at least with libev), if we have a cleanup/tearDown method
+        # that does 'del self.listener' AND we haven't sometime
+        # previously closed the listener (while the test body was executing)
+        # we tend to sometimes see hangs when tests run in succession;
+        # notably test_empty_send followed by test_makefile produces a hang
+        # in test_makefile when it tries to read from the client_file, because
+        # the accept() call in accept_once has not yet returned a new socket to
+        # write to.
+
+        # The cause *seems* to be that the listener socket in both tests gets the
+        # same fileno(); or, at least, if we don't del the listener object,
+        # we get a different fileno, and that scenario works.
+
+        # Perhaps our logic is wrong in libev_vfd in the way we use
+        # _open_osfhandle and determine we can close it?
+        self.port = self.listener.getsockname()[1]
+
+    def _setup_listener(self):
         listener = socket.socket()
         greentest.bind_and_listen(listener, ('127.0.0.1', 0))
-        self.listener = listener
-        self.port = listener.getsockname()[1]
-
-    def cleanup(self):
-        if hasattr(self, 'listener'):
-            try:
-                self.listener.close()
-            except:
-                pass
-            del self.listener
+        return listener
 
     def create_connection(self, host='127.0.0.1', port=None, timeout=None,
                           blocking=None):
@@ -77,14 +88,19 @@ class TestTCP(greentest.TestCase):
         server_exc_info = []
 
         def accept_and_read():
+            conn = None
             try:
                 conn, _ = self.listener.accept()
                 r = conn.makefile(mode='rb')
                 read_data.append(r.read())
+                r.flush()
                 r.close()
-                conn.close()
-            except:
+            except: # pylint:disable=bare-except
                 server_exc_info.append(sys.exc_info())
+            finally:
+                if conn:
+                    conn.close()
+                self.listener.close()
 
         server = Thread(target=accept_and_read)
         client = self.create_connection(**client_args)
@@ -137,7 +153,6 @@ class TestTCP(greentest.TestCase):
         self._test_sendall(data, data, client_method='send')
 
     def test_fullduplex(self):
-
         N = 100000
 
         def server():
@@ -152,6 +167,7 @@ class TestTCP(greentest.TestCase):
             self.assertEqual(result, b'hello world')
             sender.join()
             remote_client.close()
+            self.listener.close()
 
         server_thread = Thread(target=server)
         client = self.create_connection()
@@ -211,21 +227,24 @@ class TestTCP(greentest.TestCase):
                 client_sock[0][0].close()
 
     def test_makefile(self):
-
         def accept_once():
             conn, _ = self.listener.accept()
             fd = conn.makefile(mode='wb')
             fd.write(b'hello\n')
+            fd.flush()
             fd.close()
             conn.close()  # for pypy
+            self.listener.close()
 
         acceptor = Thread(target=accept_once)
         client = self.create_connection()
-        fd = client.makefile(mode='rb')
+        # Closing the socket doesn't close the file
+        client_file = client.makefile(mode='rb')
         client.close()
-        assert fd.readline() == b'hello\n'
-        assert fd.read() == b''
-        fd.close()
+        line = client_file.readline()
+        self.assertEqual(line, b'hello\n')
+        self.assertEqual(client_file.read(), b'')
+        client_file.close()
         acceptor.join()
 
     def test_makefile_timeout(self):

src/greentest/test__ssl.py

@@ -20,10 +20,10 @@ class TestSSL(test__socket.TestTCP):
     # See https://bugs.python.org/issue10272
     TIMEOUT_ERROR = getattr(socket, 'sslerror', socket.timeout)
 
-    def setUp(self):
-        greentest.TestCase.setUp(self)
-        self.listener, _raw_listener = ssl_listener(('127.0.0.1', 0), self.privfile, self.certfile)
-        self.port = self.listener.getsockname()[1]
+    def _setup_listener(self):
+        listener, raw_listener = ssl_listener(('127.0.0.1', 0), self.privfile, self.certfile)
+        self._close_on_teardown(raw_listener)
+        return listener
 
     def create_connection(self, *args, **kwargs):
         return ssl.wrap_socket(super(TestSSL, self).create_connection(*args, **kwargs))
@@ -50,26 +50,33 @@ class TestSSL(test__socket.TestTCP):
                 # on non-blocking sockets because it's a simple loop around
                 # send(). Python 2.6 doesn't have SSLWantWriteError
                 expected = getattr(ssl, 'SSLWantWriteError', ssl.SSLError)
-                self.assertRaises(expected, client.sendall, self._test_sendall_data)
+                with self.assertRaises(expected):
+                    client.sendall(self._test_sendall_data)
             finally:
                 acceptor.join()
                 client.close()
                 server_sock[0][0].close()
 
+    @greentest.ignores_leakcheck
     def test_empty_send(self):
         # Issue 719
         # Sending empty bytes with the 'send' method raises
         # ssl.SSLEOFError in the stdlib. PyPy 4.0 and CPython 2.6
         # both just raise the superclass, ssl.SSLError.
-        expected = ssl.SSLError
-        self.assertRaises(expected, self._test_sendall,
-                          b'',
-                          client_method='send')
 
+        # Ignored during leakchecks because the third or fourth iteration of the
+        # test hangs on CPython 2/posix for some reason, likely due to
+        # the use of _close_on_teardown keeping something alive longer than intended.
+        # cf test__makefile_ref
+        with self.assertRaises(ssl.SSLError):
+            super(TestSSL, self).test_empty_send()
+
+    @greentest.ignores_leakcheck
     def test_sendall_nonblocking(self):
         # Override; doesn't work with SSL sockets.
         pass
 
+    @greentest.ignores_leakcheck
     def test_connect_with_type_flags_ignored(self):
         # Override; doesn't work with SSL sockets.
         pass