Commit 3c10a0a3 authored by Kirill Smelkov's avatar Kirill Smelkov

libgolang/gevent: Fix io_read deadlock and io_read/io_write potential data corruption

When underlying pygfobj is FileObjectThread its .readinto() leads to
deadlock because it is no cooperative(*). This manifests as
test_pyx_os_pipe_cpp hang when run by gpython on windows.

-> Workaround this by reading first into intermediate buffer and then
copying data to buffer that user provided.

After this the deadlock is gone but test_pyx_os_pipe_cpp starts to fail
and crash randomly. That's because similarly to channels we need to
care and not access a buffer if it is located on stack and owning
greenlet is inactive. Because when a greenlet is inactive, its stack is
reused by another active greenlet and writing/reading to on-stack memory
accesses that second greenlet stack, corrupting it on write.

-> do the same what we do in chan operations: use intermediate on-heap
buffer to protect original user's buffer to be accesses because it might
be located on stack. That's what actually happens in
test_pyx_os_pipe_cpp where two goroutines read and write to each other
via pipe and using on-stack located buffers. And becuase on windows
pipes, like regular files, are wrapped with FileObjectThread, when
reading greenlet becomes suspended waiting for read reasul, it will be
another greenlet to run on its stack, write to another end of a pipe,
wakeup IO thread, which will write the data to requested buffer on G1
stack and oops - it was G2 there.

(*) see https://github.com/gevent/gevent/pull/1948 for details
parent d9505256
...@@ -346,7 +346,19 @@ cdef: ...@@ -346,7 +346,19 @@ cdef:
cdef uint8_t[::1] mem = <uint8_t[:count]>buf cdef uint8_t[::1] mem = <uint8_t[:count]>buf
xmem = memoryview(mem) # to avoid https://github.com/cython/cython/issues/3900 on mem[:0]=b'' xmem = memoryview(mem) # to avoid https://github.com/cython/cython/issues/3900 on mem[:0]=b''
try: try:
n = pygfobj.readinto(xmem) # NOTE buf might be on stack, so it must not be accessed, e.g. from
# FileObjectThread, while our greenlet is parked (see STACK_DEAD_WHILE_PARKED
# for details). -> Use intermediate on-heap buffer to protect from that.
#
# Also: we cannot use pygfobj.readinto due to
# https://github.com/gevent/gevent/pull/1948
#
# TODO use .readinto() when buf is not on stack and gevent is
# recent enough or pygfobj is not FileObjectThread.
#n = pygfobj.readinto(xmem)
buf2 = pygfobj.read(count)
n = len(buf2)
xmem[:n] = buf2
except OSError as e: except OSError as e:
n = -e.errno n = -e.errno
out_n[0] = n out_n[0] = n
...@@ -369,8 +381,14 @@ cdef: ...@@ -369,8 +381,14 @@ cdef:
bint _io_write(IOH* ioh, int* out_n, const void *buf, size_t count): bint _io_write(IOH* ioh, int* out_n, const void *buf, size_t count):
pygfobj = <object>ioh.pygfobj pygfobj = <object>ioh.pygfobj
cdef const uint8_t[::1] mem = <const uint8_t[:count]>buf cdef const uint8_t[::1] mem = <const uint8_t[:count]>buf
# NOTE buf might be on stack, so it must not be accessed, e.g. from
# FileObjectThread, while our greenlet is parked (see STACK_DEAD_WHILE_PARKED
# for details). -> Use intermediate on-heap buffer to protect from that.
buf2 = bytearray(mem)
try: try:
n = pygfobj.write(mem) n = pygfobj.write(buf2)
except OSError as e: except OSError as e:
n = -e.errno n = -e.errno
out_n[0] = n out_n[0] = n
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment