Skip to content

[3.13] gh-132869: Fix crash in _PyObject_TryGetInstanceAttribute #133700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2025

Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion Lib/test/test_free_threading/test_dict.py
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@

from ast import Or
from functools import partial
from threading import Thread
from threading import Thread, Barrier
from unittest import TestCase

try:
@@ -142,6 +142,25 @@ def writer_func(l):
for ref in thread_list:
self.assertIsNone(ref())

def test_getattr_setattr(self):
NUM_THREADS = 10
b = Barrier(NUM_THREADS)

def closure(b, c):
b.wait()
for i in range(10):
getattr(c, f'attr_{i}', None)
setattr(c, f'attr_{i}', 99)

class MyClass:
pass

o = MyClass()
threads = [Thread(target=closure, args=(b, o))
for _ in range(NUM_THREADS)]
with threading_helper.start_threads(threads):
pass

@unittest.skipIf(_testcapi is None, 'need _testcapi module')
def test_dict_version(self):
dict_version = _testcapi.dict_version
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix crash in the :term:`free threading` build when accessing an object
attribute that may be concurrently inserted or deleted.
58 changes: 50 additions & 8 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
@@ -1185,6 +1185,37 @@ dictkeys_generic_lookup(PyDictObject *mp, PyDictKeysObject* dk, PyObject *key, P
return do_lookup(mp, dk, key, hash, compare_generic);
}

#ifdef Py_GIL_DISABLED

static Py_ssize_t
unicodekeys_lookup_unicode_threadsafe(PyDictKeysObject* dk, PyObject *key,
Py_hash_t hash);

#endif

static Py_ssize_t
unicodekeys_lookup_split(PyDictKeysObject* dk, PyObject *key, Py_hash_t hash)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, the dictobject.c changes are taken verbatim from main

{
Py_ssize_t ix;
assert(dk->dk_kind == DICT_KEYS_SPLIT);
assert(PyUnicode_CheckExact(key));

#ifdef Py_GIL_DISABLED
/ A split dictionaries keys can be mutated by other dictionaries
/ but if we have a unicode key we can avoid locking the shared
/ keys.
ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash);
if (ix == DKIX_KEY_CHANGED) {
LOCK_KEYS(dk);
ix = unicodekeys_lookup_unicode(dk, key, hash);
UNLOCK_KEYS(dk);
}
#else
ix = unicodekeys_lookup_unicode(dk, key, hash);
#endif
return ix;
}

/* Lookup a string in a (all unicode) dict keys.
* Returns DKIX_ERROR if key is not a string,
* or if the dict keys is not all strings.
@@ -1209,13 +1240,24 @@ _PyDictKeys_StringLookup(PyDictKeysObject* dk, PyObject *key)
return unicodekeys_lookup_unicode(dk, key, hash);
}

#ifdef Py_GIL_DISABLED

static Py_ssize_t
unicodekeys_lookup_unicode_threadsafe(PyDictKeysObject* dk, PyObject *key,
Py_hash_t hash);

#endif
/* Like _PyDictKeys_StringLookup() but only works on split keys. Note
* that in free-threaded builds this locks the keys object as required.
*/
Py_ssize_t
_PyDictKeys_StringLookupSplit(PyDictKeysObject* dk, PyObject *key)
{
assert(dk->dk_kind == DICT_KEYS_SPLIT);
assert(PyUnicode_CheckExact(key));
Py_hash_t hash = unicode_get_hash(key);
if (hash == -1) {
hash = PyUnicode_Type.tp_hash(key);
if (hash == -1) {
PyErr_Clear();
return DKIX_ERROR;
}
}
return unicodekeys_lookup_split(dk, key, hash);
}

/*
The basic lookup function used by all operations.
@@ -6972,7 +7014,7 @@ _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name, PyObject **attr

PyDictKeysObject *keys = CACHED_KEYS(Py_TYPE(obj));
assert(keys != NULL);
Py_ssize_t ix = _PyDictKeys_StringLookup(keys, name);
Py_ssize_t ix = _PyDictKeys_StringLookupSplit(keys, name);
if (ix == DKIX_EMPTY) {
*attr = NULL;
return true;
Loading
Oops, something went wrong.

Follow Lee on X/Twitter - Father, Husband, Serial builder creating AI, crypto, games & web tools. We are friends :) AI Will Come To Life!

Check out: eBank.nz (Art Generator) | Netwrck.com (AI Tools) | Text-Generator.io (AI API) | BitBank.nz (Crypto AI) | ReadingTime (Kids Reading) | RewordGame | BigMultiplayerChess | WebFiddle | How.nz | Helix AI Assistant