gh-143189: fix insertdict() for non-Unicode key#143285
Uh oh!
There was an error while loading. Please reload this page.
Conversation
When iserting non unicode key into split table, matching Unicode key may be in the shared split table without its value.
methane added interpreter-core There was a problem hiding this comment.
Pull request overview
This PR fixes a crash in the CPython dictionary implementation when inserting a non-str key (specifically a str subclass) into a split table dictionary where the key matches an existing key in the shared split table but has no corresponding value in the instance dictionary yet.
Key Changes:
- Fixed the logic in
insertdict()to checkold_value == NULLinstead ofix == DKIX_EMPTYto properly handle split tables where a key exists without a value - Added explanatory comments documenting this edge case behavior
- Added test coverage for the specific scenario that was causing the crash
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Objects/dictobject.c | Fixed the condition check in insertdict() to use old_value == NULL instead of ix == DKIX_EMPTY and added clarifying comments about split table behavior |
| Misc/NEWS.d/next/Core_and_Builtins/2025-12-30-06-48-48.gh-issue-143189.in_sv2.rst | Added changelog entry describing the crash fix |
| Lib/test/test_dict.py | Added new test case test_split_table_insert_with_str_subclass to verify the fix works correctly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lib/test/test_dict.py Outdated
| obj2 = MyClass() | ||
| d = obj2.__dict__ | ||
| d[MyStr("attr1")] = 2 | ||
| assert isinstance(list(d)[0], MyStr) |
There was a problem hiding this comment.
Use unittest assertion method instead of bare assert statement. This ensures the test properly reports failures with helpful messages and follows unittest best practices. Replace with self.assertTrue(isinstance(list(d)[0], MyStr)).
| assertisinstance(list(d)[0], MyStr) | |
| self.assertTrue(isinstance(list(d)[0], MyStr)) |
hugovk merged commit 43c7658 into python:mainUh oh!
There was an error while loading. Please reload this page.
Sorry, @methane and @hugovk, I could not cleanly backport this to |
Sorry, @methane and @hugovk, I could not cleanly backport this to |
hugovk commented Jan 12, 2026
@methane Please can you check the backports? |
GH-143771 is a backport of this pull request to the 3.14 branch. |
GH-143772 is a backport of this pull request to the 3.13 branch. |
…H-143285) (cherry picked from commit 43c7658) Co-authored-by: Inada Naoki <[email protected]>
methane added the type-crash 3 participants
When iserting non unicode key into split table, matching Unicode key may be in the shared split table without its value.
insertdict#143189