Skip to content

Commit e90061f

Browse files
cmaloneyvstinner
andauthored
gh-60107: Remove a copy from RawIOBase.read (#141532)
If the underlying I/O class keeps a reference to the memory, raise BufferError. Co-authored-by: Victor Stinner <vstinner@python.org>
1 parent 722f4bb commit e90061f

File tree

7 files changed

+39
-11
lines changed

7 files changed

+39
-11
lines changed

Include/internal/pycore_global_objects_fini_generated.h

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_global_strings.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,7 @@ struct _Py_global_strings {
793793
STRUCT_FOR_ID(symmetric_difference_update)
794794
STRUCT_FOR_ID(tabsize)
795795
STRUCT_FOR_ID(tag)
796+
STRUCT_FOR_ID(take_bytes)
796797
STRUCT_FOR_ID(target)
797798
STRUCT_FOR_ID(target_is_directory)
798799
STRUCT_FOR_ID(task)

Include/internal/pycore_runtime_init_generated.h

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_unicodeobject_generated.h

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Lib/test/test_io/test_general.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,25 @@ def readinto(self, b):
609609
with self.assertRaises(ValueError):
610610
Misbehaved(bad_size).read()
611611

612+
def test_RawIOBase_read_gh60107(self):
613+
# gh-60107: Ensure a "Raw I/O" which keeps a reference to the
614+
# mutable memory doesn't allow making a mutable bytes.
615+
class RawIOKeepsReference(self.MockRawIOWithoutRead):
616+
def __init__(self, *args, **kwargs):
617+
self.buf = None
618+
super().__init__(*args, **kwargs)
619+
620+
def readinto(self, buf):
621+
# buf is the bytearray so keeping a reference to it doesn't keep
622+
# the memory alive; a memoryview does.
623+
self.buf = memoryview(buf)
624+
buf[0:4] = self._read_stack.pop()
625+
return 3
626+
627+
with self.assertRaises(BufferError):
628+
rawio = RawIOKeepsReference([b"1234"])
629+
rawio.read(4)
630+
612631
def test_types_have_dict(self):
613632
test = (
614633
self.IOBase(),
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Remove a copy from :meth:`io.RawIOBase.read`. If the underlying I/O class
2+
keeps a reference to the mutable memory, raise a :exc:`BufferError`.

Modules/_io/iobase.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -927,33 +927,33 @@ _io__RawIOBase_read_impl(PyObject *self, Py_ssize_t n)
927927
return PyObject_CallMethodNoArgs(self, &_Py_ID(readall));
928928
}
929929

930-
/* TODO: allocate a bytes object directly instead and manually construct
931-
a writable memoryview pointing to it. */
932930
b = PyByteArray_FromStringAndSize(NULL, n);
933-
if (b == NULL)
931+
if (b == NULL) {
934932
return NULL;
933+
}
935934

936935
res = PyObject_CallMethodObjArgs(self, &_Py_ID(readinto), b, NULL);
937936
if (res == NULL || res == Py_None) {
938-
Py_DECREF(b);
939-
return res;
937+
goto cleanup;
940938
}
941939

942940
Py_ssize_t bytes_filled = PyNumber_AsSsize_t(res, PyExc_ValueError);
943-
Py_DECREF(res);
941+
Py_CLEAR(res);
944942
if (bytes_filled == -1 && PyErr_Occurred()) {
945-
Py_DECREF(b);
946-
return NULL;
943+
goto cleanup;
947944
}
948945
if (bytes_filled < 0 || bytes_filled > n) {
949-
Py_DECREF(b);
950946
PyErr_Format(PyExc_ValueError,
951947
"readinto returned %zd outside buffer size %zd",
952948
bytes_filled, n);
953-
return NULL;
949+
goto cleanup;
950+
}
951+
if (PyByteArray_Resize(b, bytes_filled) < 0) {
952+
goto cleanup;
954953
}
954+
res = PyObject_CallMethodNoArgs(b, &_Py_ID(take_bytes));
955955

956-
res = PyBytes_FromStringAndSize(PyByteArray_AsString(b), bytes_filled);
956+
cleanup:
957957
Py_DECREF(b);
958958
return res;
959959
}

0 commit comments

Comments
 (0)