Skip to content

Commit 7805fa1

Browse files
committed
gh-143378: refactor the buffer check and unit tests for C and Python implementation
1 parent 89b2cb7 commit 7805fa1

File tree

3 files changed

+55
-29
lines changed

3 files changed

+55
-29
lines changed

Lib/_pyio.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,8 @@ def write(self, b):
955955
raise TypeError("can't write str to binary stream")
956956
with memoryview(b) as view:
957957
n = view.nbytes # Size of any bytes-like object
958+
if self.closed:
959+
raise ValueError("write to closed file")
958960
if n == 0:
959961
return 0
960962

Lib/test/test_io/test_memoryio.py

Lines changed: 46 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,52 @@ def test_issue5449(self):
587587
self.ioclass(initial_bytes=buf)
588588
self.assertRaises(TypeError, self.ioclass, buf, foo=None)
589589

590+
def test_write_concurrent_close(self):
591+
# Prevent crashes when memio.write() concurrently closes 'memio'.
592+
# See: https://github.com/python/cpython/issues/143378.
593+
class B:
594+
def __buffer__(self, flags):
595+
memio.close()
596+
return memoryview(b"A")
597+
598+
memio = self.ioclass()
599+
self.assertRaises(ValueError, memio.write, B())
600+
601+
def test_writelines_concurrent_close(self):
602+
# Prevent crashes when memio.writelines() concurrently closes 'memio'.
603+
# See: https://github.com/python/cpython/issues/143378.
604+
class B:
605+
def __buffer__(self, flags):
606+
memio.close()
607+
return memoryview(b"A")
608+
609+
memio = self.ioclass()
610+
self.assertRaises(ValueError, memio.writelines, [B()])
611+
612+
def test_write_concurrent_export(self):
613+
# Prevent crashes when memio.write() concurrently exports 'memio'.
614+
# See: https://github.com/python/cpython/issues/143378.
615+
class B:
616+
buf = None
617+
def __buffer__(self, flags):
618+
self.buf = memio.getbuffer()
619+
return memoryview(b"A")
620+
621+
memio = self.ioclass()
622+
self.assertRaises(BufferError, memio.write, B())
623+
624+
def test_writelines_concurrent_export(self):
625+
# Prevent crashes when memio.writelines() concurrently exports 'memio'.
626+
# See: https://github.com/python/cpython/issues/143378.
627+
class B:
628+
buf = None
629+
def __buffer__(self, flags):
630+
self.buf = memio.getbuffer()
631+
return memoryview(b"A")
632+
633+
memio = self.ioclass()
634+
self.assertRaises(BufferError, memio.writelines, [B()])
635+
590636

591637
class TextIOTestMixin:
592638

@@ -856,31 +902,6 @@ def test_cow_mutable(self):
856902
memio = self.ioclass(ba)
857903
self.assertEqual(sys.getrefcount(ba), old_rc)
858904

859-
@support.cpython_only
860-
def test_write_concurrent_mutation(self):
861-
# Prevent crashes when buf.write() concurrently mutates 'buf'.
862-
# See: https://github.com/python/cpython/issues/143378.
863-
class B:
864-
def __buffer__(self, flags):
865-
memio.close()
866-
return memoryview(b"A")
867-
868-
memio = self.ioclass()
869-
self.assertRaises(BufferError, memio.write, B())
870-
871-
@support.cpython_only
872-
def test_writelines_concurrent_mutation(self):
873-
# Prevent crashes when buf.writelines() concurrently mutates 'buf'.
874-
# See: https://github.com/python/cpython/issues/143378.
875-
class B:
876-
def __buffer__(self, flags):
877-
memio.close()
878-
return memoryview(b"A")
879-
880-
memio = self.ioclass()
881-
self.assertRaises(BufferError, memio.writelines, [B()])
882-
883-
884905
class CStringIOTest(PyStringIOTest):
885906
ioclass = io.StringIO
886907
UnsupportedOperation = io.UnsupportedOperation

Modules/_io/bytesio.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,14 +202,17 @@ write_bytes_lock_held(bytesio *self, PyObject *b)
202202
}
203203

204204
Py_buffer buf;
205-
self->exports++;
205+
Py_ssize_t len;
206206
if (PyObject_GetBuffer(b, &buf, PyBUF_CONTIG_RO) < 0) {
207-
self->exports--;
208207
return -1;
209208
}
210-
self->exports--;
211209

212-
Py_ssize_t len = buf.len;
210+
if (check_closed(self) || check_exports(self)) {
211+
len = -1;
212+
goto done;
213+
}
214+
215+
len = buf.len;
213216
if (len == 0) {
214217
goto done;
215218
}

0 commit comments

Comments
 (0)