Commits

Denis Bilenko committed dedaf2c

fileobject: use __del__ implemented in Cython so that it cannot be interrupted asynchronously; multiple bugfixes

- add 'close' argument to SocketAdapter
- handle EINTR
- remove _flushlock
- remove closedfileobject

  • Participants
  • Parent commits feb0bfa

Comments (0)

Files changed (2)

File gevent/_util.pyx

 from python cimport *
+import os
 
 
 __all__ = ['set_exc_info']
         Py_INCREF(<PyObjectPtr>value)
         tstate.exc_value = <PyObjectPtr>value
     tstate.exc_traceback = NULL
+
+
+# We implement __del__s in Cython so that they are safe against signals
+
+
+def SocketAdapter__del__(self, close=os.close):
+    fileno = self._fileno
+    if fileno is not None:
+        self._fileno = None
+        if self._close:
+            close(fileno)
+
+
+def noop(self):
+    pass

File gevent/fileobject.py

 import sys
 import os
 from gevent.hub import get_hub
-from gevent.lock import RLock
 from gevent.socket import EBADF
 
 
 
 else:
 
-    from gevent.socket import _fileobject, EAGAIN
+    from gevent.socket import _fileobject, EAGAIN, _get_memory
+    from errno import EINTR
     cancel_wait_ex = IOError(EBADF, 'File descriptor was closed in another greenlet')
 
+    try:
+        from gevent._util import SocketAdapter__del__, noop
+    except ImportError:
+        SocketAdapter__del__ = None
+        noop = None
+
+    from types import UnboundMethodType
+
+
+    class NA(object):
+
+        def __repr__(self):
+            return 'N/A'
+
+    NA = NA()
+
 
     class SocketAdapter(object):
         """Socket-like API on top of a file descriptor.
         from file descriptors on POSIX platforms.
         """
 
-        def __init__(self, fileno, mode=None):
+        def __init__(self, fileno, mode=None, close=True):
             if not isinstance(fileno, (int, long)):
                 raise TypeError('fileno must be int: %r' % fileno)
             self._fileno = fileno
             self._mode = mode or 'rb'
-            self._translate = 'U' in mode
+            self._close = close
+            self._translate = 'U' in self._mode
             fcntl(fileno, F_SETFL, os.O_NONBLOCK)
             self._eat_newline = False
             self.hub = get_hub()
 
         def __repr__(self):
             if self._fileno is None:
-                return '<%s closed>' % (self.__class__.__name__, )
+                return '<%s at 0x%x closed>' % (self.__class__.__name__, id(self))
             else:
-                return '%s(%r, %r)' % (self.__class__.__name__, self._fileno, self._mode)
+                args = (self.__class__.__name__, id(self), getattr(self, '_fileno', NA), getattr(self, '_mode', NA))
+                return '<%s at 0x%x (%r, %r)>' % args
 
         def makefile(self, *args, **kwargs):
             return _fileobject(self, *args, **kwargs)
                 raise IOError(EBADF, 'Bad file descriptor (%s object is closed)' % self.__class__.__name)
             return result
 
+        def detach(self):
+            x = self._fileno
+            self._fileno = None
+            return x
+
         def close(self):
             self.hub.cancel_wait(self._read_event, cancel_wait_ex)
             self.hub.cancel_wait(self._write_event, cancel_wait_ex)
             fileno = self._fileno
             if fileno is not None:
                 self._fileno = None
-                os.close(fileno)
+                if self._close:
+                    os.close(fileno)
 
         def sendall(self, data):
             fileno = self.fileno()
                 try:
                     data = os.read(self.fileno(), size)
                 except (IOError, OSError):
-                    ex = sys.exc_info()[1]
-                    if ex.args[0] == EBADF:
+                    code = sys.exc_info()[1].args[0]
+                    if code == EBADF:
                         return ''
-                    if ex.args[0] != EAGAIN:
+                    elif code == EINTR:
+                        sys.exc_clear()
+                        continue
+                    elif code != EAGAIN:
                         raise
                     sys.exc_clear()
                 else:
             data = data.replace("\r", "\n")
             return data
 
+        if not SocketAdapter__del__:
+
+            def __del__(self, close=os.close):
+                fileno = self._fileno
+                if fileno is not None:
+                    close(fileno)
+
+    if SocketAdapter__del__:
+        SocketAdapter.__del__ = UnboundMethodType(SocketAdapter__del__, None, SocketAdapter)
+
 
     class FileObjectPosix(_fileobject):
 
                 fobj = None
             else:
                 fileno = fobj.fileno()
-            sock = SocketAdapter(fileno, mode)
+            sock = SocketAdapter(fileno, mode, close=close)
             self._fobj = fobj
-            self._flushlock = RLock()
+            self._closed = False
             _fileobject.__init__(self, sock, mode=mode, bufsize=bufsize, close=close)
 
         def __repr__(self):
                 return '<%s %s _fobj=%r>' % (self.__class__.__name__, self._sock, self._fobj)
 
         def close(self):
+            if self._closed:
+                # make sure close() is only ran once when called concurrently
+                # cannot rely on self._sock for this because we need to keep that until flush() is done
+                return
+            self._closed = True
             sock = self._sock
             if sock is None:
                 return
             try:
                 self.flush()
             finally:
+                if self._fobj is not None or not self._close:
+                    sock.detach()
                 self._sock = None
-                if self._close:
-                    sock.close()
                 self._fobj = None
-                # what if fobj has its own close() in __del__ like fdopen?
-
-        def flush(self):
-            # the reason we make flush() greenlet-safe is to make close() greenlet-safe
-            with self._flushlock:
-                return _fileobject.flush(self)
 
         def __getattr__(self, item):
             assert item != '_fobj'
+            if self._fobj is None:
+                raise FileObjectClosed
             return getattr(self._fobj, item)
 
+        if not noop:
+
+            def __del__(self):
+                pass
+
+    if noop:
+        FileObjectPosix.__del__ = UnboundMethodType(FileObjectPosix, None, noop)
+
 
 class FileObjectThreadPool(object):
 
         if threadpool is None:
             threadpool = get_hub().threadpool
         self.threadpool = threadpool
-        self._flushlock = RLock()
 
     def close(self):
         fobj = self._fobj
-        if fobj is closedfileobject:
+        if fobj is None:
             return
+        self._fobj = None
         try:
-            self.flush()
+            self.flush(__fobj=fobj)
         finally:
-            self._fobj = closedfileobject
             if self._close:
                 fobj.close()
 
-    def flush(self):
-        # the reason we make flush() greenlet-safe is to make close() greenlet-safe
-        fobj = self._fobj
-        if fobj is closedfileobject:
-            raise closedfileobject
-        with self._flushlock:
-            return self.threadpool.apply_e(BaseException, self._fobj.flush)
-
-    def __del__(self):
-        try:
-            self.close()
-        except:
-            # close() may fail if __init__ didn't complete
-            pass
+    def flush(self, __fobj=None):
+        if __fobj is not None:
+            fobj = __fobj
+        else:
+            fobj = self._fobj
+        if fobj is None:
+            raise FileObjectClosed
+        return self.threadpool.apply_e(BaseException, fobj.flush)
 
     def __repr__(self):
         return '<%s _fobj=%r threadpool=%r>' % (self.__class__.__name__, self._fobj, self.threadpool)
 
     def __getattr__(self, item):
         assert item != '_fobj'
+        if self._fobj is None:
+            raise FileObjectClosed
         return getattr(self._fobj, item)
 
     for method in ['read', 'readinto', 'readline', 'readlines', 'write', 'writelines', 'xreadlines']:
 
         exec '''def %s(self, *args, **kwargs):
     fobj = self._fobj
-    if fobj is closedfileobject:
-        raise closedfileobject
+    if fobj is None:
+        raise FileObjectClosed
     return self.threadpool.apply_e(BaseException, fobj.%s, args, kwargs)''' % (method, method)
 
     def __iter__(self):
         raise StopIteration
 
 
-class closedfileobject(IOError):
-    
-    def __init__(self):
-        IOError.__init__(self, EBADF, 'Bad file descriptor (FileObjectThreadPool was closed)')
-
-    def _dummy(self, *args, **kwargs):
-        raise self
-
-    __getattr__ = _dummy
-    __call__ = _dummy
-
-closedfileobject = closedfileobject()
+FileObjectClosed = IOError(EBADF, 'Bad file descriptor (FileObject was closed)')
 
 
 try: