Simplify reraising logic for GreenFileDescriptorIO.seek.

Don't introduce a reference cycle with the traceback.

In the test, don't use sys.stdout as a non-seekable stream; putting it into non-blocking mode messes up the console. Instead create a pipe for this purpose.

Simplify test cases by splitting FOPosix and FOThread.

CHANGES.rst

@@ -67,6 +67,10 @@
   `UserWarning` when using the libuv backend. Reported in
   :issue:`1321` by ZeroNet.
 
+- Fix ``FileObjectPosix.seek`` raising `OSError` when it should have
+  been `IOError` on Python 2. Reported by, and PR by, Ricardo Kirkner.
+  See :issue:`1323`.
+
 1.3.7 (2018-10-12)
 ==================
 

src/gevent/_fileobjectposix.py

@@ -9,7 +9,7 @@ from io import DEFAULT_BUFFER_SIZE
 from io import RawIOBase
 from io import UnsupportedOperation
 
-from gevent._compat import PY3, reraise
+from gevent._compat import reraise
 from gevent._fileobjectcommon import cancel_wait_ex
 from gevent._fileobjectcommon import FileObjectBase
 from gevent.hub import get_hub
@@ -144,14 +144,15 @@ class GreenFileDescriptorIO(RawIOBase):
     def seek(self, offset, whence=0):
         try:
             return os.lseek(self._fileno, offset, whence)
-        except OSError as ex:
-            if not PY3:
-                # Python 2.x
-                # make sure on Python 2.x we raise an IOError
-                exc_info = sys.exc_info()
-                reraise(IOError, IOError(*ex.args), tb=exc_info[2])
-            # otherwise just re-raise the original exception
+        except IOError: # pylint:disable=try-except-raise
             raise
+        except OSError as ex: # pylint:disable=duplicate-except
+            # Python 2.x
+            # make sure on Python 2.x we raise an IOError
+            # as documented for RawIOBase.
+            # See https://github.com/gevent/gevent/issues/1323
+            reraise(IOError, IOError(*ex.args), sys.exc_info()[2])
+
 
 class FlushingBufferedWriter(BufferedWriter):
 
@@ -160,6 +161,7 @@ class FlushingBufferedWriter(BufferedWriter):
         self.flush()
         return ret
 
+
 class FileObjectPosix(FileObjectBase):
     """
     A file-like object that operates on non-blocking files but

src/gevent/tests/test__fileobject.py

@@ -6,14 +6,13 @@ import gc
 import unittest
 
 import gevent
-from gevent.fileobject import FileObject, FileObjectThread
+from gevent import fileobject
 
 import gevent.testing as greentest
 from gevent.testing.sysinfo import PY3
 from gevent.testing.flaky import reraiseFlakyTestRaceConditionLibuv
 from gevent.testing.skipping import skipOnLibuvOnCIOnPyPy
 from gevent.testing.skipping import skipOnWindows
-from gevent.testing.skipping import skipOnPy37
 
 try:
     ResourceWarning
@@ -22,7 +21,20 @@ except NameError:
         "Python 2 fallback"
 
 
-class Test(greentest.TestCase):
+def writer(fobj, line):
+    for character in line:
+        fobj.write(character)
+        fobj.flush()
+    fobj.close()
+
+
+class TestFileObjectThread(greentest.TestCase):
+
+    def _getTargetClass(self):
+        return fileobject.FileObjectThread
+
+    def _makeOne(self, *args, **kwargs):
+        return self._getTargetClass()(*args, **kwargs)
 
     def _test_del(self, **kwargs):
         pipe = os.pipe()
@@ -37,7 +49,7 @@ class Test(greentest.TestCase):
 
     def _do_test_del(self, pipe, **kwargs):
         r, w = pipe
-        s = FileObject(w, 'wb', **kwargs)
+        s = self._makeOne(w, 'wb', **kwargs)
         s.write(b'x')
         try:
             s.flush()
@@ -61,7 +73,7 @@ class Test(greentest.TestCase):
         else:
             os.close(w)
 
-        with FileObject(r, 'rb') as fobj:
+        with self._makeOne(r, 'rb') as fobj:
             self.assertEqual(fobj.read(), b'x')
 
     # We only use FileObjectThread on Win32. Sometimes the
@@ -81,29 +93,25 @@ class Test(greentest.TestCase):
     def test_del_close(self):
         self._test_del(close=True)
 
-    if FileObject is not FileObjectThread:
-        # FileObjectThread uses os.fdopen() when passed a file-descriptor, which returns
-        # an object with a destructor that can't be bypassed, so we can't even
-        # create one that way
-        def test_del_noclose(self):
+    # FileObjectThread uses os.fdopen() when passed a file-descriptor, which returns
+    # an object with a destructor that can't be bypassed, so we can't even
+    # create one that way
+    def test_del_noclose(self):
+        with self.assertRaisesRegex(TypeError,
+                                    'FileObjectThread does not support close=False on an fd.'):
             self._test_del(close=False)
-    else:
-        def test_del_noclose(self):
-            with self.assertRaisesRegex(TypeError,
-                                        'FileObjectThread does not support close=False on an fd.'):
-                self._test_del(close=False)
 
     def test_newlines(self):
         import warnings
         r, w = os.pipe()
         lines = [b'line1\n', b'line2\r', b'line3\r\n', b'line4\r\nline5', b'\nline6']
-        g = gevent.spawn(writer, FileObject(w, 'wb'), lines)
+        g = gevent.spawn(writer, self._makeOne(w, 'wb'), lines)
 
         try:
             with warnings.catch_warnings():
                 warnings.simplefilter('ignore', DeprecationWarning)
                 # U is deprecated in Python 3, shows up on FileObjectThread
-                fobj = FileObject(r, 'rU')
+                fobj = self._makeOne(r, 'rU')
             result = fobj.read()
             fobj.close()
             self.assertEqual('line1\nline2\nline3\nline4\nline5\nline6', result)
@@ -127,7 +135,7 @@ class Test(greentest.TestCase):
 
         with open(path, 'rb') as f_raw:
             try:
-                f = FileObject(f_raw, 'rb')
+                f = self._makeOne(f_raw, 'rb')
             except ValueError:
                 # libuv on Travis can raise EPERM
                 # from FileObjectPosix. I can't produce it on mac os locally,
@@ -136,7 +144,7 @@ class Test(greentest.TestCase):
                 # That shouldn't have any effect on io watchers, though, which were
                 # already being explicitly closed.
                 reraiseFlakyTestRaceConditionLibuv()
-            if PY3 or FileObject is not FileObjectThread:
+            if PY3 or not isinstance(f, fileobject.FileObjectThread):
                 self.assertTrue(f.seekable())
             f.seek(15)
             self.assertEqual(15, f.tell())
@@ -145,38 +153,19 @@ class Test(greentest.TestCase):
         self.assertEqual(native_data, s)
         self.assertEqual(native_data, fileobj_data)
 
-    @skipOnPy37
-    @skipOnWindows("FileObject not used on Win32")
-    def test_seek_raises_ioerror(self):
-        # Issue #1323
-        fd = sys.stdout
-        with self.assertRaises(OSError) as ctx:
-            os.lseek(fd.fileno(), 0, 0)
-        os_ex = ctx.exception
-
-        with self.assertRaises(IOError) as ctx:
-            f = FileObject(fd, 'r')
-            f.seek(0)
-        io_ex = ctx.exception
-
-        self.assertEqual(io_ex.args, os_ex.args)
-        self.assertEqual(io_ex.errno, os_ex.errno)
-        self.assertEqual(io_ex.strerror, os_ex.strerror)
-        self.assertEqual(str(io_ex), str(os_ex))
-
     def test_close_pipe(self):
         # Issue #190, 203
         r, w = os.pipe()
-        x = FileObject(r)
-        y = FileObject(w, 'w')
+        x = self._makeOne(r)
+        y = self._makeOne(w, 'w')
         x.close()
         y.close()
 
     def test_read1(self):
         # Issue #840
         r, w = os.pipe()
-        x = FileObject(r)
-        y = FileObject(w, 'w')
+        x = self._makeOne(r)
+        y = self._makeOne(w, 'w')
         self._close_on_teardown(x)
         self._close_on_teardown(y)
         self.assertTrue(hasattr(x, 'read1'))
@@ -185,8 +174,8 @@ class Test(greentest.TestCase):
     def test_bufsize_0(self):
         # Issue #840
         r, w = os.pipe()
-        x = FileObject(r, 'rb', bufsize=0)
-        y = FileObject(w, 'wb', bufsize=0)
+        x = self._makeOne(r, 'rb', bufsize=0)
+        y = self._makeOne(w, 'wb', bufsize=0)
         self._close_on_teardown(x)
         self._close_on_teardown(y)
         y.write(b'a')
@@ -197,11 +186,44 @@ class Test(greentest.TestCase):
         b = x.read(1)
         self.assertEqual(b, b'2')
 
-def writer(fobj, line):
-    for character in line:
-        fobj.write(character)
-        fobj.flush()
-    fobj.close()
+
+@unittest.skipUnless(
+    hasattr(fileobject, 'FileObjectPosix'),
+    "Needs FileObjectPosix"
+)
+class TestFileObjectPosix(TestFileObjectThread):
+
+    def _getTargetClass(self):
+        return fileobject.FileObjectPosix
+
+    def test_del_noclose(self):
+        self._test_del(close=False)
+
+    def test_seek_raises_ioerror(self):
+        # https://github.com/gevent/gevent/issues/1323
+
+        # Get a non-seekable file descriptor
+        r, w = os.pipe()
+
+        self.addCleanup(os.close, r)
+        self.addCleanup(os.close, w)
+
+        with self.assertRaises(OSError) as ctx:
+            os.lseek(r, 0, os.SEEK_SET)
+        os_ex = ctx.exception
+
+        with self.assertRaises(IOError) as ctx:
+            f = self._makeOne(r, 'r', close=False)
+            # Seek directly using the underlying GreenFileDescriptorIO;
+            # the buffer may do different things, depending
+            # on the version of Python (especially 3.7+)
+            f.fileio.seek(0)
+        io_ex = ctx.exception
+
+        self.assertEqual(io_ex.errno, os_ex.errno)
+        self.assertEqual(io_ex.strerror, os_ex.strerror)
+        self.assertEqual(io_ex.args, os_ex.args)
+        self.assertEqual(str(io_ex), str(os_ex))
 
 
 class TestTextMode(unittest.TestCase):