Repository navigation
[C API] _PyTuple_Resize() must copy the tuple if the hash value was already computed #161226
Description
Recently, issue gh-158219 was found in bytearray(str, encoding) if a codec computes somehow the hash value of the created bytes object. I wrote PR gh-158329 to not use a bytes object (as the internal bytearray buffer) if it's the hash value was already computed. Instead, the bytes string is copied. With commit 969af80, _PyBytes_IsMutable() now returns false if the hash value was already computed.
_PyBytes_Resize() was recently changed to create a copy if the hash value was already computed: see issue gh-158585 and PR gh-158589 (commit 6e7738b).
PyUnicode_Resize() creates a copy if _PyUnicode_IsModifiable() is false, and _PyUnicode_IsModifiable() returns false if the hash value was already computed since commit 488fa49 (2011).
IMO we should also modify _PyTuple_Resize() to return a copy if tuple hash value was already computed.
In CPython code, _PyTuple_Resize() is only called in a few functions, and none of these functions compute the hash value of the tuple before resizing it. So this change will not impact the performance of CPython (stdlib). Functions calling _PyTuple_Resize():
- Modules/_tkinter.c:
FlattenContextAPI - Modules/itertoolsmodule.c:
batched_next() - Objects/codeobject.c:
_PyTuple_Resize(&localsplusnames, nlocalsplus) - Objects/genericaliasobject.c:
tuple_extend(),_Py_make_parameters() - Objects/structseq.c:
initialize_structseq_dict()
Activity
pythongh-161226: Copy the tuple in _PyTuple_Resize() if the hash is c…
gvanrossum commentedon Nov 3, 2026
Why not just clear the computed hash code from the existing tuple?
Copying seems more work, and it should be extremely rare to find a tuple's hash already computed when resizing it (IMO it would always be a bug): resizing a tuple should only ever happen while we're building a tuple from variable-length data (e.g. from an iterator), and before the tuple is exposed in any way to any other part of the code besides the code doing the building/resizing.
(But hmm, that reasoning would also apply to PyUnicode_Resize(). Why didn't it apply there?)
pythongh-161226: Copy the tuple in _PyTuple_Resize() if the hash is c…
pythongh-161226: Copy the tuple in _PyTuple_Resize() if the hash is c…
pythongh-161226: Copy the tuple in _PyTuple_Resize() if the hash is c…
vstinner commentedon Nov 3, 2026
@gvanrossum:
Same rationale than
_PyBytes_Resize()andPyUnicode_Resize(): it's not safe to modify a tuple in-place if its hash value was already computed; it's a sign that it was already used by Python somehow. It's safer to return a copy in this case.Note:
_PyTuple_Resize()fails withSystemErrorif the reference count is not1(if_PyObject_IsUniquelyReferenced()is false).As I wrote in my previous message, CPython is not impacted by this issue. So the change is only for 3rd party C extensions which call
_PyTuple_Resize()on a tuple with cached hash value. My concern is to have a well defined behavior in_PyTuple_Resize()in all cases.One issue of
PyTuple_New()and_PyTuple_New()is that the tuple is immediately tracked by the garbage collector, so Python can discover the tuple which is being build using functions likegc.get_objects(). It caused bugs in the past. But well, it's another issue, unrelated to this one.By the way,
PyTuple_SetItem()andPyTuple_SET_ITEM()accept modifying a tuple with a reference count greater than 1 and/or with cached hash value. There are a few functions in CPython which rely on this to modify an existing tuple in-place. For example, search for code calling_PyTuple_Recycle()(which resets the hash value).vstinner commentedon Nov 3, 2026
Well, raising
SystemErrorif the hash value is already computed is another option. The problem is that it may break C extensions relying on the old behavior.If there are C extensions impacted by this issue, it may be nicer to them to just return a copy, rather than failing with an exception.
gvanrossum commentedon Nov 3, 2026
I just worry that we're adding so much code for something that shouldn't be happening. A quick raise would be better, given that the invalid situation can be forced via get_objects()?