https://github.com/python/cpython/commit/e2ff498cc03886b73fca0429ee745b04646ad8e0
commit: e2ff498cc03886b73fca0429ee745b04646ad8e0
branch: main
author: Victor Stinner <[email protected]>
committer: vstinner <[email protected]>
date: 2026-09-18T02:43:38Z
summary:
gh-157242: Add byteswriter_check_consistency() (#157723)
Add byteswriter_check_consistency() to check that the bytes/bytearray
object can be mutated. It is called by all PyBytesWriter functions.
* Add test_thread to PyBytesWriter tests.
* _PyBytes_IsMutable() now checks _PyObject_IsUniquelyReferenced().
* Clarify thread safety in PyBytesWriter documentation.
files:
M Doc/c-api/bytes.rst
M Include/internal/pycore_unicodeobject.h
M Lib/test/test_capi/test_bytes.py
M Objects/bytesobject.c
diff --git a/Doc/c-api/bytes.rst b/Doc/c-api/bytes.rst
index c03816b5727fa4..fbdad0f346550d 100644
--- a/Doc/c-api/bytes.rst
+++ b/Doc/c-api/bytes.rst
@@ -305,8 +305,8 @@ object.
A bytes writer object.
- The API is **not thread safe**. A :c:type:`PyBytesWriter` object must only
- be used by a single thread, it must not be shared between threads.
+ The API is **not thread safe**. To share a writer with multiple threads, a
+ critical section or a lock is needed.
The instance must be destroyed by :c:func:`PyBytesWriter_Finish` on
success, or :c:func:`PyBytesWriter_Discard` on error.
diff --git a/Include/internal/pycore_unicodeobject.h
b/Include/internal/pycore_unicodeobject.h
index acd6bcf0813f9f..e9a4aed37030e7 100644
--- a/Include/internal/pycore_unicodeobject.h
+++ b/Include/internal/pycore_unicodeobject.h
@@ -116,7 +116,8 @@ _PyUnicodeWriter_CanWrite(_PyUnicodeWriter *writer)
PyObject *buffer = writer->buffer;
assert(buffer != NULL);
// Do not use _PyObject_IsUniquelyReferenced(): the caller can have its own
- // lock to prevent a writer being used by two theads at the same time.
+ // lock to prevent a writer from being used by two threads at the same
+ // time.
assert(Py_REFCNT(buffer) == 1);
assert(PyUnstable_Unicode_GET_CACHED_HASH(buffer) == -1);
assert(!PyUnicode_CHECK_INTERNED(buffer));
diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py
index b68412a02b3228..1356e5d6c51c14 100644
--- a/Lib/test/test_capi/test_bytes.py
+++ b/Lib/test/test_capi/test_bytes.py
@@ -1,8 +1,10 @@
import sys
import textwrap
+import threading
import unittest
from test import support
from test.support import import_helper
+from test.support import threading_helper
from test.support.script_helper import assert_python_failure
_testlimitedcapi = import_helper.import_module('_testlimitedcapi')
@@ -461,6 +463,14 @@ def test_resize(self):
self.assertEqual(writer.finish(),
b's' * small + b'L' * (large - small))
+ # Make sure that it's possible to write after a resize to zero
+ # when a bytes/bytearray object is allocated.
+ writer = self.create_writer()
+ writer.resize(self.LARGE_BUFFER)
+ writer.resize(0)
+ writer.write_bytes(b'abc', 3)
+ self.assertEqual(writer.finish(), b'abc')
+
# invalid size
for size in (self.SMALL_BUFFER, self.LARGE_BUFFER):
with self.subTest(size=size):
@@ -665,6 +675,36 @@ def test_get_data_canary(self):
self.assertEqual(get_data_canary(writer),
b'abc123' + CANARY_BYTE)
+ @threading_helper.requires_working_threading()
+ def test_thread(self):
+ # PyBytesWriter can be used by multiple threads: it's up to the caller
+ # to implement a lock to prevent concurrent accesses.
+ writer = self.create_writer(0)
+ size = None
+ data = None
+
+ def thread_func(writer, LARGE_BUFFER):
+ nonlocal size, data
+
+ # create a bytes object for the buffer
+ writer.write_bytes(b'x' * LARGE_BUFFER, LARGE_BUFFER)
+
+ # so we can check a write with a bytes object
+ writer.write_bytes(b'yz', 2)
+ writer.format_i(b'i=%i', 5)
+ writer.resize(10)
+ data = writer.get_data()
+ size = writer.get_size()
+
+ thread = threading.Thread(target=thread_func,
+ args=(writer, self.LARGE_BUFFER))
+ thread.start()
+ threading_helper.join_thread(thread)
+
+ self.assertEqual(size, 10)
+ self.assertEqual(data, b'x' * 10)
+ self.assertEqual(writer.finish(), b'x' * 10)
+
class BytesWriterTest(BaseWriterTest, unittest.TestCase):
RESULT_TYPE = bytes
diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c
index 38ec7a4aefcf56..7fefd64eefb2ac 100644
--- a/Objects/bytesobject.c
+++ b/Objects/bytesobject.c
@@ -3329,18 +3329,22 @@ PyBytes_ConcatAndDel(PyObject **pv, PyObject *w)
//
// Usage: assert(_PyBytes_IsMutable(obj)).
int
-_PyBytes_IsMutable(PyObject *v)
+_PyBytes_IsMutable(PyObject *self)
{
- // Singleton objects must never be modified
- assert(!_Py_IsImmortal(v));
+ assert(PyBytes_Check(self));
+ // Do not use _PyObject_IsUniquelyReferenced(): this function is called
+ // by bytearray and PyBytesWriter which can be used by multiple threads.
+ assert(Py_REFCNT(self) == 1);
+ assert(!_Py_IsImmortal(self));
- Py_ssize_t size = PyBytes_GET_SIZE(v);
+ // Check that the object is not a singleton
+ Py_ssize_t size = PyBytes_GET_SIZE(self);
if (size == 0) {
- assert(v != bytes_get_empty());
+ assert(self != bytes_get_empty());
}
else if (size == 1) {
- unsigned char ch = PyBytes_AS_STRING(v)[0];
- assert(v != (PyObject*)CHARACTER(ch));
+ unsigned char ch = PyBytes_AS_STRING(self)[0];
+ assert(self != (PyObject*)CHARACTER(ch));
}
return 1;
}
@@ -3675,20 +3679,6 @@ byteswriter_allocated(PyBytesWriter *writer)
#ifdef Py_DEBUG
-static void
-byteswriter_check_canary_byte(PyBytesWriter *writer)
-{
- const unsigned char *data = (const unsigned char*)byteswriter_data(writer);
- unsigned char canary = data[writer->size];
- if (canary != PyBytesWriter_CANARY_BYTE) {
- _Py_FatalErrorFormat(__func__,
- "Buffer overflow detected in PyBytesWriter %p "
- "at position %zd",
- writer, writer->size);
- }
-}
-
-
static void
byteswriter_write_canary_byte(PyBytesWriter *writer)
{
@@ -3710,6 +3700,45 @@ byteswriter_reset_trailing_byte(PyBytesWriter *writer)
#endif
+#ifndef NDEBUG
+static int
+byteswriter_check_consistency(PyBytesWriter *writer)
+{
+ PyObject *obj = writer->obj;
+ if (obj != NULL) {
+ if (writer->use_bytearray) {
+ assert(PyByteArray_CheckExact(obj));
+ // Do not use _PyObject_IsUniquelyReferenced(): the caller can have
+ // its own lock to prevent a writer from being used by two threads
+ // at the same time.
+ assert(Py_REFCNT(obj) == 1);
+ PyByteArrayObject *bytearray = (PyByteArrayObject*)obj;
+ obj = bytearray->ob_bytes_object;
+ assert(obj != NULL);
+ }
+
+ // Code adapted from _PyBytes_IsMutable()
+ assert(PyBytes_CheckExact(obj));
+ assert(_PyBytes_IsMutable(obj));
+ // -1 since the last small buffer byte is used as the canary byte
+ assert((size_t)PyBytes_GET_SIZE(obj) > (sizeof(writer->small_buffer) -
1));
+ }
+
+#ifdef Py_DEBUG
+ const unsigned char *data = (const unsigned char*)byteswriter_data(writer);
+ unsigned char canary = data[writer->size];
+ if (canary != PyBytesWriter_CANARY_BYTE) {
+ _Py_FatalErrorFormat(__func__,
+ "Buffer overflow detected in PyBytesWriter %p "
+ "at position %zd",
+ writer, writer->size);
+ }
+#endif
+ return 1;
+}
+#endif
+
+
#ifdef MS_WINDOWS
/* On Windows, overallocate by 50% is the best factor */
# define OVERALLOCATE_FACTOR 2
@@ -3743,6 +3772,7 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t
new_size, int resize)
// bytearray can override the canary byte on error
byteswriter_write_canary_byte(writer);
#endif
+ assert(byteswriter_check_consistency(writer));
return -1;
}
}
@@ -3750,6 +3780,7 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t
new_size, int resize)
// Can raise MemoryError or OverflowError
if (_PyBytes_ResizeKeepOnError(&writer->obj, alloc)) {
assert(writer->obj != NULL);
+ assert(byteswriter_check_consistency(writer));
return -1;
}
assert(_PyBytes_IsMutable(writer->obj));
@@ -3821,7 +3852,7 @@ byteswriter_create(Py_ssize_t size, int use_bytearray)
if (size >= 1) {
if (byteswriter_resize(writer, size, 0) < 0) {
#ifdef Py_DEBUG
- // Write the canary byte so byteswriter_check_canary_byte()
+ // Write the canary byte so byteswriter_check_consistency()
// doesn't fail in PyBytesWriter_Discard()
byteswriter_write_canary_byte(writer);
#endif
@@ -3835,6 +3866,7 @@ byteswriter_create(Py_ssize_t size, int use_bytearray)
byteswriter_allocated(writer));
byteswriter_write_canary_byte(writer);
#endif
+ assert(byteswriter_check_consistency(writer));
return writer;
}
@@ -3858,8 +3890,8 @@ PyBytesWriter_Discard(PyBytesWriter *writer)
return;
}
+ assert(byteswriter_check_consistency(writer));
#ifdef Py_DEBUG
- byteswriter_check_canary_byte(writer);
if (writer->obj != NULL) {
byteswriter_reset_trailing_byte(writer);
}
@@ -3873,6 +3905,8 @@ PyBytesWriter_Discard(PyBytesWriter *writer)
PyObject*
PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size)
{
+ assert(byteswriter_check_consistency(writer));
+
// Check for negative size here to raise ValueError in all cases, rather
// than having a different exception depending on the code path. For
// example, _PyBytes_Resize() raises SystemError on negative size.
@@ -3886,10 +3920,6 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer,
Py_ssize_t size)
goto error;
}
-#ifdef Py_DEBUG
- byteswriter_check_canary_byte(writer);
-#endif
-
PyObject *result;
if (size == 0) {
result = bytes_get_empty();
@@ -3938,7 +3968,7 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer,
Py_ssize_t size)
}
#ifdef Py_DEBUG
- // Reset the writer, so byteswriter_check_canary_byte() doesn't fail
+ // Reset the writer, so byteswriter_check_consistency() doesn't fail
// in PyBytesWriter_Discard().
writer->size = 0;
byteswriter_write_canary_byte(writer);
@@ -3970,9 +4000,7 @@ PyBytesWriter_FinishWithPointer(PyBytesWriter *writer,
void *buf)
void*
PyBytesWriter_GetData(PyBytesWriter *writer)
{
-#ifdef Py_DEBUG
- byteswriter_check_canary_byte(writer);
-#endif
+ assert(byteswriter_check_consistency(writer));
return byteswriter_data(writer);
}
@@ -3981,9 +4009,7 @@ PyBytesWriter_GetData(PyBytesWriter *writer)
Py_ssize_t
PyBytesWriter_GetSize(PyBytesWriter *writer)
{
-#ifdef Py_DEBUG
- byteswriter_check_canary_byte(writer);
-#endif
+ assert(byteswriter_check_consistency(writer));
return _PyBytesWriter_GetSize(writer);
}
@@ -3992,9 +4018,7 @@ PyBytesWriter_GetSize(PyBytesWriter *writer)
int
PyBytesWriter_Resize(PyBytesWriter *writer, Py_ssize_t new_size)
{
-#ifdef Py_DEBUG
- byteswriter_check_canary_byte(writer);
-#endif
+ assert(byteswriter_check_consistency(writer));
if (new_size < 0) {
PyErr_SetString(PyExc_ValueError, "size must be >= 0");
@@ -4012,6 +4036,7 @@ PyBytesWriter_Resize(PyBytesWriter *writer, Py_ssize_t
new_size)
#ifdef Py_DEBUG
byteswriter_write_canary_byte(writer);
#endif
+ assert(byteswriter_check_consistency(writer));
return 0;
}
@@ -4031,9 +4056,7 @@ _PyBytesWriter_ResizeAndUpdatePointer(PyBytesWriter
*writer, Py_ssize_t size,
int
PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t grow)
{
-#ifdef Py_DEBUG
- byteswriter_check_canary_byte(writer);
-#endif
+ assert(byteswriter_check_consistency(writer));
if (grow == 0) {
// Nothing to do
@@ -4064,6 +4087,7 @@ PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t grow)
#ifdef Py_DEBUG
byteswriter_write_canary_byte(writer);
#endif
+ assert(byteswriter_check_consistency(writer));
return 0;
}
@@ -4099,6 +4123,8 @@ PyBytesWriter_WriteBytes(PyBytesWriter *writer,
}
char *buf = byteswriter_data(writer);
memcpy(buf + pos, bytes, size);
+
+ assert(byteswriter_check_consistency(writer));
return 0;
}
@@ -4127,14 +4153,12 @@ PyBytesWriter_Format(PyBytesWriter *writer, const char
*format, ...)
static Py_ssize_t
_PyBytesWriter_ResizeToAllocated(PyBytesWriter *writer)
{
-#ifdef Py_DEBUG
- byteswriter_check_canary_byte(writer);
-#endif
-
Py_ssize_t allocated = byteswriter_allocated(writer);
writer->size = allocated;
#ifdef Py_DEBUG
byteswriter_write_canary_byte(writer);
#endif
+
+ assert(byteswriter_check_consistency(writer));
return allocated;
}
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3//lists/python-checkins.python.org
Member address: [email protected]
