From beb37144d1bbed3c72fd8c348dec3f88b3598cad Mon Sep 17 00:00:00 2001 From: Stan Ulbrych Date: Thu, 10 Sep 2026 10:10:18 +0100 Subject: [PATCH] gh-156995: Fix `bytearray.take_bytes()` corrupting shared single-byte singletons (GH-156996) (cherry picked from commit fd569ea94120815b01baf8bcbcf21e8e09102db6) Co-authored-by: Stan Ulbrych Co-authored-by: Cody Maloney Co-authored-by: Victor Stinner --- Lib/test/test_bytes.py | 21 ++++++++ Lib/test/test_capi/test_bytes.py | 52 ++++++++++++------- ...-09-05-17-32-11.gh-issue-156995.Tk3bYt.rst | 5 ++ Objects/bytearrayobject.c | 13 +++-- Objects/bytesobject.c | 14 +++-- 5 files changed, 75 insertions(+), 30 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-09-05-17-32-11.gh-issue-156995.Tk3bYt.rst diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py index 720b38cb508cbe..c714fc2e76e83c 100644 --- a/Lib/test/test_bytes.py +++ b/Lib/test/test_bytes.py @@ -1611,6 +1611,14 @@ def test_take_bytes(self): self.assertRaises(BufferError, ba.take_bytes) self.assertEqual(ba.take_bytes(), b'abc') + # Leaving one byte must not adopt the shared single-byte bytes object + # as the buffer. + ba = bytearray(b'abc') + self.assertEqual(ba.take_bytes(2), b'ab') + ba[0] = ord('A') + self.assertEqual(ba, bytearray(b'A')) + self.assertEqual(ord(b'c'), ord('c')) + @support.cpython_only # tests an implementation detail def test_take_bytes_optimization(self): # Validate optimization around taking lots of little chunks out of a @@ -3031,5 +3039,18 @@ def resize_stress(ba): with threading_helper.start_threads(threads): pass + @threading_helper.reap_threads + @threading_helper.requires_working_threading() + def test_free_threading_bytearray_resize_other_thread(self): + # Shrinking a bytearray whose buffer another thread owns must not + # adopt the immortal single-byte bytes object a the buffer. + ba = bytearray(b'abc') + thread = threading.Thread(target=ba.resize, args=(1,)) + with threading_helper.start_threads([thread]): + pass + ba[0] = ord('X') + self.assertEqual(ba, bytearray(b'X')) + self.assertEqual(ord(b'a'), ord('a')) + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index d20e5016f969c2..0f2360c90c82c8 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -1,3 +1,4 @@ +import sys import unittest from test.support import import_helper @@ -231,26 +232,41 @@ def test_decodeescape(self): def test_resize(self): """Test _PyBytes_Resize()""" - resize = _testcapi.bytes_resize + _resize = _testcapi.bytes_resize + + def resize(obj, size, new): + result = _resize(obj, size, new) + if 1 <= len(result): + if new or size != len(obj): + # gh-156995: Make sure that the result is a fresh object. + # Previously, _PyBytes_Resize(&obj, 1) returned a singleton + # if _PyObject_IsUniquelyReferenced() is false. + self.assertEqual(sys.getrefcount(result), 1) + self.assertFalse(sys._is_immortal(result)) + else: + # check that the result is the empty bytes string singleton + self.assertTrue(sys._is_immortal(result)) + return result for new in True, False: - self.assertEqual(resize(b'abc', 0, new), b'') - self.assertEqual(resize(b'abc', 1, new), b'a') - self.assertEqual(resize(b'abc', 2, new), b'ab') - self.assertEqual(resize(b'abc', 3, new), b'abc') - b = resize(b'abc', 4, new) - self.assertEqual(len(b), 4) - self.assertEqual(b[:3], b'abc') - - self.assertEqual(resize(b'a', 0, new), b'') - self.assertEqual(resize(b'a', 1, new), b'a') - b = resize(b'a', 2, new) - self.assertEqual(len(b), 2) - self.assertEqual(b[:1], b'a') - - self.assertEqual(resize(b'', 0, new), b'') - self.assertEqual(len(resize(b'', 1, new)), 1) - self.assertEqual(len(resize(b'', 2, new)), 2) + with self.subTest(new=new): + self.assertEqual(resize(b'abc', 0, new), b'') + self.assertEqual(resize(b'abc', 1, new), b'a') + self.assertEqual(resize(b'abc', 2, new), b'ab') + self.assertEqual(resize(b'abc', 3, new), b'abc') + b = resize(b'abc', 4, new) + self.assertEqual(len(b), 4) + self.assertEqual(b[:3], b'abc') + + self.assertEqual(resize(b'a', 0, new), b'') + self.assertEqual(resize(b'a', 1, new), b'a') + b = resize(b'a', 2, new) + self.assertEqual(len(b), 2) + self.assertEqual(b[:1], b'a') + + self.assertEqual(resize(b'', 0, new), b'') + self.assertEqual(len(resize(b'', 1, new)), 1) + self.assertEqual(len(resize(b'', 2, new)), 2) self.assertRaises(SystemError, resize, b'abc', -1, False) self.assertRaises(SystemError, resize, bytearray(b'abc'), 3, False) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-09-05-17-32-11.gh-issue-156995.Tk3bYt.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-05-17-32-11.gh-issue-156995.Tk3bYt.rst new file mode 100644 index 00000000000000..8d5970e7f4a374 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-05-17-32-11.gh-issue-156995.Tk3bYt.rst @@ -0,0 +1,5 @@ +Fix :class:`bytearray` sharing its buffer with the single-byte :class:`bytes` +object of the same value, so that writing to the bytearray modified that +:class:`bytes` object. This happened with :meth:`bytearray.take_bytes` when +exactly one byte remained, and on the free-threaded build when a bytearray was +shrunk to one byte from another thread. diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index b16fb8751f5f73..e5024b1c74682d 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -45,7 +45,10 @@ _getbytevalue(PyObject* arg, int *value) static void bytearray_reinit_from_bytes(PyByteArrayObject *self, Py_ssize_t size, - Py_ssize_t alloc) { + Py_ssize_t alloc) +{ + /* Only the empty bytes may be immortal. */ + assert((alloc == 0) == _Py_IsImmortal(self->ob_bytes_object)); self->ob_bytes = self->ob_start = PyBytes_AS_STRING(self->ob_bytes_object); Py_SET_SIZE(self, size); FT_ATOMIC_STORE_SSIZE_RELAXED(self->ob_alloc, alloc); @@ -1616,12 +1619,14 @@ bytearray_take_bytes_impl(PyByteArrayObject *self, PyObject *n) return ret; } - // Copy remaining bytes to a new bytes. - PyObject *remaining = PyBytes_FromStringAndSize(self->ob_start + to_take, - remaining_length); + // Copy remaining bytes to a new bytes. Allocate and then copy + // so we don't get a shared immortal one-character singleton! + PyObject *remaining = PyBytes_FromStringAndSize(NULL, remaining_length); if (remaining == NULL) { return NULL; } + memcpy(PyBytes_AS_STRING(remaining), self->ob_start + to_take, + remaining_length); // If the bytes are offset inside the buffer must first align. if (self->ob_start != self->ob_bytes) { diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 5fa20b9f6a4aaa..1f0b03beed43b1 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3368,14 +3368,12 @@ _PyBytes_Resize(PyObject **pv, Py_ssize_t newsize) return 0; } if (!_PyObject_IsUniquelyReferenced(v)) { - if (oldsize < newsize) { - *pv = _PyBytes_FromSize(newsize, 0); - if (*pv) { - memcpy(PyBytes_AS_STRING(*pv), PyBytes_AS_STRING(v), oldsize); - } - } - else { - *pv = PyBytes_FromStringAndSize(PyBytes_AS_STRING(v), newsize); + // Allocate and then copy so we don't get a shared immortal + // one-character singleton! + *pv = _PyBytes_FromSize(newsize, 0); + if (*pv) { + memcpy(PyBytes_AS_STRING(*pv), PyBytes_AS_STRING(v), + Py_MIN(oldsize, newsize)); } Py_DECREF(v); return (*pv == NULL) ? -1 : 0;