Down to one failure, stdlib test_ssl ThreadedTests.test_asyncore_server.

Not clear why.

CHANGES.rst

@@ -20,6 +20,21 @@ Library and Dependency Updates
 - Upgrade libev from 4.25 to 4.31 and update its embedded
   ``config.guess`` to the latest.
 
+  .. important::
+
+     libev, when built with ``EV_VERIFY >= 2``, now performs
+     verification of file descriptors when IO watchers are started
+     *and* when they are stopped. If you first close a file descriptor
+     and only then stop an associated watcher, libev will abort the
+     process.
+
+     Using the standard gevent socket and file objects handles this
+     automatically, but if you're using the IO watchers directly,
+     you'll need to watch out for this.
+
+     The binary wheels gevent distributes *do not* set ``EV_VERIFY``
+     and don't have this issue.
+
 1.5a3 (2020-01-01)
 ==================
 

src/gevent/_fileobjectposix.py

@@ -109,10 +109,14 @@ class GreenFileDescriptorIO(RawIOBase):
         hub = self.hub
         self.hub = self._read_watcher = self._write_watcher = None
 
-        if read_event is not None:
-            hub.cancel_wait(read_event, cancel_wait_ex, True)
-        if write_event is not None:
-            hub.cancel_wait(write_event, cancel_wait_ex, True)
+        hub.cancel_waits_close_and_then(
+            (read_event, write_event),
+            cancel_wait_ex,
+            self.__finish_close,
+            self._closefd,
+            self._fileno,
+            self._keep_alive
+        )
 
     def close(self):
         if self._closed:
@@ -121,12 +125,15 @@ class GreenFileDescriptorIO(RawIOBase):
         # TODO: Can we use 'read_event is not None and write_event is
         # not None' to mean _closed?
         self._closed = True
-        self.__destroy_events()
-        fileno = self._fileno
-        keep_alive = self._keep_alive
-        self._fileno = self._keep_alive = None
         try:
-            if self._closefd:
+            self.__destroy_events()
+        finally:
+            self._fileno = self._keep_alive = None
+
+    @staticmethod
+    def __finish_close(closefd, fileno, keep_alive):
+        try:
+            if closefd:
                 os.close(fileno)
         finally:
             if hasattr(keep_alive, 'close'):

src/gevent/_hub_primitives.py

@@ -66,6 +66,25 @@ class WaitOperationsGreenlet(SwitchOutGreenletWithLoop): # pylint:disable=undefi
         finally:
             watcher.stop()
 
+    def cancel_waits_close_and_then(self, watchers, exc_kind, then, *then_args):
+        deferred = []
+        for watcher in watchers:
+            if watcher is None:
+                continue
+            if watcher.callback is None:
+                watcher.close()
+            else:
+                deferred.append(watcher)
+        if deferred:
+            self.loop.run_callback(self._cancel_waits_then, deferred, exc_kind, then, then_args)
+        else:
+            then(*then_args)
+
+    def _cancel_waits_then(self, watchers, exc_kind, then, then_args):
+        for watcher in watchers:
+            self._cancel_wait(watcher, exc_kind, True)
+        then(*then_args)
+
     def cancel_wait(self, watcher, error, close_watcher=False):
         """
         Cancel an in-progress call to :meth:`wait` by throwing the given *error*
@@ -77,17 +96,25 @@ class WaitOperationsGreenlet(SwitchOutGreenletWithLoop): # pylint:disable=undefi
            be discarded. Closing the watcher is important to release native resources.
         .. versionchanged:: 1.3a2
            Allow the *watcher* to be ``None``. No action is taken in that case.
+
         """
         if watcher is None:
             # Presumably already closed.
             # See https://github.com/gevent/gevent/issues/1089
             return
+
         if watcher.callback is not None:
             self.loop.run_callback(self._cancel_wait, watcher, error, close_watcher)
-        elif close_watcher:
+            return
+
+        if close_watcher:
             watcher.close()
 
     def _cancel_wait(self, watcher, error, close_watcher):
+        # Running in the hub. Switches to the waiting greenlet to raise
+        # the error; assuming the waiting greenlet dies, switches back
+        # to this  (because the waiting greenlet's parent is the hub.)
+
         # We have to check again to see if it was still active by the time
         # our callback actually runs.
         active = watcher.active

src/gevent/_socket2.py

@@ -57,6 +57,9 @@ else:
     # very ugly and fragile.
 
     class _fileobject(_fileobject): # pylint:disable=function-redefined
+        __slots__ = (
+            '__weakref__',
+        )
 
         def __enter__(self):
             return self
@@ -67,14 +70,14 @@ else:
 
         def close(self):
             if self._sock is not None:
-                self._sock._drop_events()
+                self._sock._drop_events_and_close(closefd=False)
             super(_fileobject, self).close()
 
 
 from gevent._greenlet_primitives import get_memory as _get_memory
 
 class _closedsocket(object):
-    __slots__ = []
+    __slots__ = ()
 
     def _dummy(*args, **kwargs): # pylint:disable=no-method-argument,unused-argument
         raise error(EBADF, 'Bad file descriptor')
@@ -102,7 +105,7 @@ gtype = type
 
 from gevent._hub_primitives import wait_on_socket as _wait_on_socket
 
-class socket(object):
+class socket(_socketcommon.SocketMixin):
     """
     gevent `socket.socket <https://docs.python.org/2/library/socket.html#socket-objects>`_
     for Python 2.
@@ -117,6 +120,8 @@ class socket(object):
 
     # pylint:disable=too-many-public-methods
 
+    # TODO: Define __slots__.
+
     def __init__(self, family=AF_INET, type=SOCK_STREAM, proto=0, _sock=None):
         timeout = _socket.getdefaulttimeout()
         if _sock is None:
@@ -205,14 +210,10 @@ class socket(object):
             client_socket._drop()
         return sockobj, address
 
-    def _drop_events(self, cancel_wait_ex=cancel_wait_ex):
-        if self._read_event is not None:
-            self.hub.cancel_wait(self._read_event, cancel_wait_ex, True)
-            self._read_event = None
-        if self._write_event is not None:
-            self.hub.cancel_wait(self._write_event, cancel_wait_ex, True)
-            self._write_event = None
 
+    def _drop_ref_on_close(self, sock):
+        if PYPY:
+            sock._drop()
 
     def close(self, _closedsocket=_closedsocket):
         if not self._sock:
@@ -220,7 +221,12 @@ class socket(object):
 
         # This function should not reference any globals. See Python issue #808164.
 
-        # First, change self._sock. On CPython, this drops a
+        # First, break any reference to the loop.io objects. Our
+        # fileno, which they were tied to, is about to be free to be
+        # reused, so these objects are no longer functional.
+        self._drop_events_and_close()
+
+        # Next, change self._sock. On CPython, this drops a
         # reference, and if it was the last reference, __del__ will
         # close it. (We cannot close it, makefile() relies on
         # reference counting like this, and it may be shared among
@@ -229,19 +235,8 @@ class socket(object):
         # self._write_event/self._read_event, or they will be out of
         # sync and we may get inappropriate errors. (See
         # test__hub:TestCloseSocketWhilePolling for an example).
-        s = self._sock
         self._sock = _closedsocket()
 
-        # On PyPy we have to manually tell it to drop a ref.
-        if PYPY:
-            s._drop()
-
-        # Finally, break any reference to the loop.io objects. Our
-        # fileno, which they were tied to, is about to be free to be
-        # reused, so these objects are no longer functional.
-        self._drop_events()
-
-
     @property
     def closed(self):
         return isinstance(self._sock, _closedsocket)

src/gevent/_socket3.py

@@ -98,7 +98,7 @@ class _wrefsocket(_socket.socket):
 
 from gevent._hub_primitives import wait_on_socket as _wait_on_socket
 
-class socket(object):
+class socket(_socketcommon.SocketMixin):
     """
     gevent `socket.socket <https://docs.python.org/3/library/socket.html#socket-objects>`_
     for Python 3.
@@ -306,23 +306,13 @@ class socket(object):
         if self._closed:
             self.close()
 
-    def _drop_events(self, cancel_wait_ex=cancel_wait_ex):
-        if self._read_event is not None:
-            self.hub.cancel_wait(self._read_event, cancel_wait_ex, True)
-            self._read_event = None
-        if self._write_event is not None:
-            self.hub.cancel_wait(self._write_event, cancel_wait_ex, True)
-            self._write_event = None
+    def _drop_ref_on_close(self, sock):
+        sock.close()
 
     def _detach_socket(self, reason):
         if not self._sock:
             return
 
-        # Break any reference to the loop.io objects. Our fileno,
-        # which they were tied to, is about to be free to be reused, so these
-        # objects are no longer functional.
-        self._drop_events()
-
         # Break any references to the underlying socket object. Tested
         # by test__refcount. (Why does this matter?). Be sure to
         # preserve our same family/type/proto if possible (if we
@@ -341,6 +331,10 @@ class socket(object):
             fileno = sock.fileno()
         except OSError:
             pass
+        # Break any reference to the loop.io objects. Our fileno,
+        # which they were tied to, is about to be free to be reused, so these
+        # objects are no longer functional.
+        self._drop_events_and_close(closefd=(reason == 'closed'))
 
         self._sock = _closedsocket(family, type, proto, fileno, reason)
 
@@ -349,11 +343,8 @@ class socket(object):
         if not self._sock:
             return
 
-        sock = self._sock
-        try:
-            self._detach_socket('closed')
-        finally:
-            sock.close()
+        self._detach_socket('closed')
+
 
     def close(self):
         # This function should not reference any globals. See Python issue #808164.

src/gevent/_socketcommon.py

@@ -405,3 +405,27 @@ def _resolve_addr(sock, address):
     else:
         address = (address[0], port, address[2], address[3])
     return address
+
+class SocketMixin(object):
+    __slots__ = (
+        '_read_event',
+        '_write_event',
+        '_sock',
+    )
+
+    def _drop_events_and_close(self, closefd=True, _cancel_wait_ex=cancel_wait_ex):
+        hub = self.hub
+        read_event = self._read_event
+        write_event = self._write_event
+        self._read_event = self._write_event = None
+        hub.cancel_waits_close_and_then(
+            (read_event, write_event),
+            _cancel_wait_ex,
+            # Pass the socket to keep it alive until such time as
+            # the waiters are guaranteed to be closed.
+            self._drop_ref_on_close if closefd else id,
+            self._sock
+        )
+
+    def _drop_ref_on_close(self, sock):
+        raise NotImplementedError

src/gevent/_sslgte279.py

@@ -635,7 +635,7 @@ class SSLSocket(socket):
         SSL channel, and the address of the remote client."""
 
         newsock, addr = socket.accept(self)
-        newsock._drop_events()
+        newsock._drop_events_and_close(closefd=False) # Why, again?
         newsock = self._context.wrap_socket(newsock,
                                             do_handshake_on_connect=self.do_handshake_on_connect,
                                             suppress_ragged_eofs=self.suppress_ragged_eofs,

src/gevent/libev/libev_vfd.h

@@ -206,7 +206,7 @@ done:
 /*
  * If libev on win32 is not embedded in gevent, then
  * the only way to map vfds is to use the default of
- * using runtime fds in libev. Note that it will leak
+ * using C runtime fds in libev. Note that it will leak
  * fds, because there's no way of closing them safely
  */
 #define vfd_get(fd) _get_osfhandle((fd))

src/gevent/testing/sysinfo.py

@@ -163,4 +163,4 @@ def libev_supports_linux_aio():
     from platform import system
     from platform import release
 
-    return system == 'Linux' and LooseVersion(release() or '0') >= LooseVersion('4.19')
+    return system() == 'Linux' and LooseVersion(release() or '0') >= LooseVersion('4.19')

src/gevent/tests/test__select.py

@@ -71,10 +71,11 @@ class TestPollRead(gevent.testing.timing.AbstractGenericWaitTestCase):
         poll = select.poll()
         self.assertRaises(KeyError, poll.unregister, 5)
 
-    @unittest.skipIf(hasattr(gevent.core, 'libuv'),
-                     "Depending on whether the fileno is reused or not this either crashes or does nothing."
-                     "libuv won't open a watcher for a closed file on linux.")
     def test_poll_invalid(self):
+        self.skipTest(
+            "libev >= 4.27 aborts the process if built with EV_VERIFY >= 2. "
+            "For libuv, depending on whether the fileno is reused or not "
+            "this either crashes or does nothing.")
         with open(__file__, 'rb') as fp:
             fd = fp.fileno()
 

src/gevent/tests/test__server.py

@@ -106,7 +106,7 @@ class TestCase(greentest.TestCase):
         return server_host, self.server.server_port, family
 
     @contextmanager
-    def makefile(self, timeout=_DEFAULT_SOCKET_TIMEOUT, bufsize=1):
+    def makefile(self, timeout=_DEFAULT_SOCKET_TIMEOUT, bufsize=1, include_raw_socket=False):
         server_host, server_port, family = self.get_server_host_port_family()
         bufarg = 'buffering' if PY3 else 'bufsize'
         makefile_kwargs = {bufarg: bufsize}
@@ -117,20 +117,14 @@ class TestCase(greentest.TestCase):
 
         with socket.socket(family=family) as sock:
             rconn = None
+            # We want the socket to be accessible from the fileobject
+            # we return. On Python 2, natively this is available as
+            # _sock, but Python 3 doesn't have that.
             sock.connect((server_host, server_port))
             sock.settimeout(timeout)
             with sock.makefile(**makefile_kwargs) as rconn:
-                # We want the socket to be accessible from the fileobject
-                # we return. On Python 2, natively this is available as
-                # _sock, but Python 3 doesn't have that.
-                # We emulate it by assigning to a new attribute; note that this
-                # (probably) introduces a cycle on Python 3, so we are careful
-                # to clear it.
-                rconn.gevent_sock = sock
-                try:
-                    yield rconn
-                finally:
-                    del rconn.gevent_sock
+                result = rconn if not include_raw_socket else (rconn, sock)
+                yield result
 
     def send_request(self, url='/', timeout=_DEFAULT_SOCKET_TIMEOUT, bufsize=1):
         with self.makefile(timeout=timeout, bufsize=bufsize) as conn:
@@ -161,13 +155,13 @@ class TestCase(greentest.TestCase):
         self.Settings.assertPoolFull(self)
 
     def assertNotAccepted(self):
-        with self.makefile() as conn:
+        with self.makefile(include_raw_socket=True) as (conn, sock):
             conn.write(b'GET / HTTP/1.0\r\n\r\n')
             conn.flush()
             result = b''
             try:
                 while True:
-                    data = conn.gevent_sock.recv(1)
+                    data = sock.recv(1)
                     if not data:
                         break
                     result += data