Skip to content

Commit 60e956e

Browse files
authored
gh-149816: Fix race between setattr and object.__dict__ update (#152296)
Fix race between `setattr` and `object.__dict__` update (#152296)
1 parent 9d42aee commit 60e956e

3 files changed

Lines changed: 72 additions & 5 deletions

File tree

‎Lib/test/test_free_threading/test_dict.py‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,5 +437,61 @@ def writer():
437437

438438
threading_helper.run_concurrently([reader, writer])
439439

440+
def test_racing_dict_update_and_method_lookup_with_inline_values(self):
441+
# gh-149816: sub-case 108
442+
# The race below checks that a detached dict is still valid
443+
# when racing setattr with __dict__ replacement
444+
class Target:
445+
pass
446+
447+
def appender(obj: Target, start: Barrier, iter_times: int) -> None:
448+
start.wait()
449+
index = 0
450+
for _ in range(iter_times):
451+
setattr(obj, f"probe_{index}", index)
452+
index += 1
453+
time.sleep(0)
454+
455+
def replacer(obj: Target, start: Barrier, iter_times: int, churn_size: int) -> None:
456+
start.wait()
457+
for _ in range(iter_times):
458+
old_dict = obj.__dict__
459+
obj.__dict__ = {}
460+
del old_dict
461+
time.sleep(0)
462+
# create a list of dicts to trigger a realloc of the dict's table
463+
# and ensure that the old dict is not used after it is deleted
464+
realloc_trigger_list = [{"k": j} for j in range(churn_size)]
465+
del realloc_trigger_list
466+
467+
def race(iter_times: int,
468+
churn_size: int,
469+
appender_threads: int,
470+
replacer_threads: int) -> None:
471+
obj = Target()
472+
setattr(obj, "origin", 0)
473+
_ = obj.__dict__ # Access __dict__ to ensure it's initialized
474+
475+
start = Barrier(appender_threads + replacer_threads)
476+
threads = []
477+
for _ in range(appender_threads):
478+
threads.append(Thread(target=appender, args=(obj, start, iter_times),
479+
name="appender"))
480+
for _ in range(replacer_threads):
481+
threads.append(Thread(target=replacer, args=(obj, start, iter_times, churn_size),
482+
name="replacer"))
483+
484+
with threading_helper.catch_threading_exception() as cm:
485+
with threading_helper.start_threads(threads):
486+
pass
487+
if cm.exc_type is not None:
488+
raise cm.exc_value
489+
490+
ITER_TIMES = 2_000
491+
APPENDER_THREADS = 8
492+
REPLACER_THREADS = 8
493+
CHURN_SIZE = 512
494+
race(ITER_TIMES, CHURN_SIZE, APPENDER_THREADS, REPLACER_THREADS)
495+
440496
if __name__ == "__main__":
441497
unittest.main()
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a race in free-threaded builds where storing an instance attribute could
2+
use a stale borrowed reference to an instance ``__dict__`` after another thread
3+
replaced it.

‎Objects/dictobject.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7572,17 +7572,25 @@ _PyObject_StoreInstanceAttribute(PyObject *obj, PyObject *name, PyObject *value)
75727572
{
75737573
PyDictValues *values = _PyObject_InlineValues(obj);
75747574
if (!FT_ATOMIC_LOAD_UINT8(values->valid)) {
7575-
PyDictObject *dict = _PyObject_GetManagedDict(obj);
7575+
PyDictObject *dict;
7576+
int res;
7577+
#ifdef Py_GIL_DISABLED
7578+
Py_BEGIN_CRITICAL_SECTION(obj);
7579+
#endif
7580+
dict = _PyObject_GetManagedDict(obj);
7581+
Py_XINCREF(dict);
7582+
#ifdef Py_GIL_DISABLED
7583+
Py_END_CRITICAL_SECTION();
7584+
#endif
75767585
if (dict == NULL) {
75777586
dict = (PyDictObject *)PyObject_GenericGetDict(obj, NULL);
75787587
if (dict == NULL) {
75797588
return -1;
75807589
}
7581-
int res = store_instance_attr_dict(obj, dict, name, value);
7582-
Py_DECREF(dict);
7583-
return res;
75847590
}
7585-
return store_instance_attr_dict(obj, dict, name, value);
7591+
res = store_instance_attr_dict(obj, dict, name, value);
7592+
Py_DECREF(dict);
7593+
return res;
75867594
}
75877595

75887596
#ifdef Py_GIL_DISABLED

0 commit comments

Comments
 (0)