Skip to content

Commit

Permalink
Fix FileObjectThread.readinto to be cooperative
Browse files Browse the repository at this point in the history
.readinto was forgotten to be wrapped by FileObjectBase, and without the
wrapping FileObjectThread.readinto was calling into e.g.  RawIO.readinto
directly - instead of via threadpool - leading to deadlocks if e.g. that
readinto would need to read data from a pipe produced by another greenlet.

The fix is simple: add `readinto` into the list of wrapped methods in
FileObjectBase. Without the fix added `test_readinto` hangs for
TestFileObjectThread.

NOTE FileObjectPosix.readinto was working ok even without wrapping
because it implements cooperation by another way - via explicitly
`make_nonblocking` provided file descriptors.

/cc @jamadden
  • Loading branch information
navytux authored and jamadden committed Jul 10, 2023
1 parent 7484e95 commit bc40651
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/gevent/_fileobjectcommon.py
Expand Up @@ -469,6 +469,7 @@ class FileObjectBase(object):
'readline',
'readlines',
'read1',
'readinto',

# Write.
# Note that we do not extend WriteallMixin,
Expand Down
38 changes: 38 additions & 0 deletions src/gevent/tests/test__fileobject.py
Expand Up @@ -299,8 +299,24 @@ def check(arg):
with io.open(path) as nf:
check(nf)

@skipUnlessWorksWithRegularFiles
def test_readinto_serial(self):
fileno, path = self._mkstemp('.gevent_test_readinto')
os.write(fileno, b'hello world')
os.close(fileno)

with self._makeOne(path, 'rb') as f:
buf = bytearray(32)
mbuf = memoryview(buf)
def assertReadInto(n, dataok):
n_ = f.readinto(mbuf[:n])
self.assertEqual(n_, len(dataok))
self.assertEqual(buf[:n_], dataok)

assertReadInto(2, b'he')
assertReadInto(1, b'l')
assertReadInto(32, b'lo world')
assertReadInto(32, b'')


class ConcurrentFileObjectMixin(object):
Expand Down Expand Up @@ -371,6 +387,28 @@ def test_newlines(self):
finally:
g.kill()

def test_readinto(self):
# verify that .readinto() is cooperative.
# if .readinto() is not cooperative spawned greenlet will not be able
# to run and call to .readinto() will block forever.
r, w = self._pipe()
rf = self._makeOne(r, 'rb')
wf = self._makeOne(w, 'wb')
g = gevent.spawn(Writer, wf, [b'hello'])

try:
buf1 = bytearray(32)
buf2 = bytearray(32)

n1 = rf.readinto(buf1)
n2 = rf.readinto(buf2)

self.assertEqual(n1, 5)
self.assertEqual(buf1[:n1], b'hello')
self.assertEqual(n2, 0)
finally:
g.kill()


class TestFileObjectThread(ConcurrentFileObjectMixin, # pylint:disable=too-many-ancestors
TestFileObjectBlock):
Expand Down

0 comments on commit bc40651

Please sign in to comment.