Skip to content

Commit dc3d5ae

Browse files
DinoVDinoV
committed
Fixes
1 parent 57f7ee6 commit dc3d5ae

File tree

4 files changed

+61
-63
lines changed

4 files changed

+61
-63
lines changed

‎Lib/test/test_free_threading/test_dict.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ class MyDict(dict): pass
152152
THREAD_COUNT = 10
153153

154154
def writer_func(l):
155-
for i in range(100000):
155+
for i in range(1000):
156156
if cyclic:
157157
other_d = {}
158158
d = MyDict({"foo": 100})

‎Objects/dictobject.c‎

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -7150,7 +7150,8 @@ try_set_dict_inline_only_or_other_dict(PyObject *obj, PyObject *new_dict, PyDict
71507150
(PyDictObject *)Py_XNewRef(new_dict));
71517151
replaced = true;
71527152
goto exit_lock;
7153-
} else {
7153+
}
7154+
else {
71547155
// We have inline values, we need to lock the dict and the object
71557156
// at the same time to safely dematerialize them. To do that while releasing
71567157
// the object lock we need a strong reference to the current dictionary.
@@ -7166,38 +7167,37 @@ try_set_dict_inline_only_or_other_dict(PyObject *obj, PyObject *new_dict, PyDict
71667167
// and replaced it with another dictionary though.
71677168
static int
71687169
replace_dict_probably_inline_materialized(PyObject *obj, PyDictObject *inline_dict,
7169-
PyObject *new_dict, bool clear,
7170-
PyDictObject **replaced_dict)
7170+
PyDictObject *cur_dict, PyObject *new_dict)
71717171
{
7172-
// But we could have had another thread race in after we released
7173-
// the object lock
7174-
int err = 0;
7175-
*replaced_dict = _PyObject_GetManagedDict(obj);
7176-
assert(FT_ATOMIC_LOAD_PTR_RELAXED(inline_dict->ma_values) == _PyObject_InlineValues(obj));
7172+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj);
7173+
7174+
if (cur_dict == inline_dict) {
7175+
assert(FT_ATOMIC_LOAD_PTR_RELAXED(inline_dict->ma_values) == _PyObject_InlineValues(obj));
71777176

7178-
if (*replaced_dict == inline_dict) {
7179-
err = _PyDict_DetachFromObject(inline_dict, obj);
7177+
int err = _PyDict_DetachFromObject(inline_dict, obj);
71807178
if (err != 0) {
71817179
return err;
71827180
}
7183-
// We incref'd the inline dict and the object owns a ref.
7184-
// Clear the object's reference, we'll clear the local
7185-
// reference after releasing the lock.
7186-
if (clear) {
7187-
Py_XDECREF((PyObject *)*replaced_dict);
7188-
} else {
7189-
_PyObject_XDecRefDelayed((PyObject *)*replaced_dict);
7190-
}
7191-
*replaced_dict = NULL;
71927181
}
71937182

71947183
FT_ATOMIC_STORE_PTR(_PyObject_ManagedDictPointer(obj)->dict,
7195-
(PyDictObject *)Py_XNewRef(new_dict));
7196-
return err;
7184+
(PyDictObject *)Py_XNewRef(new_dict));
7185+
return 0;
71977186
}
71987187

71997188
#endif
72007189

7190+
static void
7191+
decref_maybe_delay(PyObject *obj, bool delay)
7192+
{
7193+
if (delay) {
7194+
_PyObject_XDecRefDelayed(obj);
7195+
}
7196+
else {
7197+
Py_XDECREF(obj);
7198+
}
7199+
}
7200+
72017201
static int
72027202
set_or_clear_managed_dict(PyObject *obj, PyObject *new_dict, bool clear)
72037203
{
@@ -7213,32 +7213,37 @@ set_or_clear_managed_dict(PyObject *obj, PyObject *new_dict, bool clear)
72137213
// values. We need to lock both the object and the dict at the
72147214
// same time to safely replace it. We can't merely lock the dictionary
72157215
// while the object is locked because it could suspend the object lock.
7216-
PyDictObject *replaced_dict;
7216+
PyDictObject *cur_dict;
72177217

72187218
assert(prev_dict != NULL);
72197219
Py_BEGIN_CRITICAL_SECTION2(obj, prev_dict);
72207220

7221-
err = replace_dict_probably_inline_materialized(obj, prev_dict, new_dict,
7222-
clear, &replaced_dict);
7221+
// We could have had another thread race in between the call to
7222+
// try_set_dict_inline_only_or_other_dict where we locked the object
7223+
// and when we unlocked and re-locked the dictionary.
7224+
cur_dict = _PyObject_GetManagedDict(obj);
7225+
7226+
err = replace_dict_probably_inline_materialized(obj, prev_dict,
7227+
cur_dict, new_dict);
72237228

72247229
Py_END_CRITICAL_SECTION2();
72257230

7226-
Py_DECREF(prev_dict);
7231+
// Decref for the dictionary we incref'd in try_set_dict_inline_only_or_other_dict
7232+
// while the object was locked
7233+
decref_maybe_delay((PyObject *)prev_dict,
7234+
!clear && prev_dict != cur_dict);
72277235
if (err != 0) {
72287236
return err;
72297237
}
7230-
prev_dict = replaced_dict;
7238+
7239+
prev_dict = cur_dict;
72317240
}
72327241

72337242
if (prev_dict != NULL) {
7234-
// Readers from the old dictionary use a borrowed reference. We need
7235-
// to set the decref the dict at the next safe point.
7236-
if (clear) {
7237-
Py_XDECREF((PyObject *)prev_dict);
7238-
} else {
7239-
_PyObject_XDecRefDelayed((PyObject *)prev_dict);
7240-
}
7243+
// decref for the dictionary that we replaced
7244+
decref_maybe_delay((PyObject *)prev_dict, !clear);
72417245
}
7246+
72427247
return 0;
72437248
#else
72447249
PyDictObject *dict = _PyObject_GetManagedDict(obj);
@@ -7265,11 +7270,7 @@ set_or_clear_managed_dict(PyObject *obj, PyObject *new_dict, bool clear)
72657270
(PyDictObject *)Py_XNewRef(new_dict));
72667271

72677272
Py_END_CRITICAL_SECTION();
7268-
if (clear) {
7269-
Py_XDECREF((PyObject *)dict);
7270-
} else {
7271-
_PyObject_XDecRefDelayed((PyObject *)dict);
7272-
}
7273+
decref_maybe_delay((PyObject *)dict, !clear);
72737274
}
72747275
assert(_PyObject_InlineValuesConsistencyCheck(obj));
72757276
return err;

‎Objects/obmalloc.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ static void
10961096
free_work_item(uintptr_t ptr, delayed_dealloc_cb cb, void *state)
10971097
{
10981098
if (ptr & 0x01) {
1099-
PyObject *obj = (PyObject*)(char *)(ptr - 1);
1099+
PyObject *obj = (PyObject *)(ptr - 1);
11001100
#ifdef Py_GIL_DISABLED
11011101
if (cb == NULL) {
11021102
assert(!_PyInterpreterState_GET()->stoptheworld.world_stopped);

‎Python/gc_free_threading.c‎

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,23 @@ gc_visit_thread_stacks(PyInterpreterState *interp)
392392
HEAD_UNLOCK(&_PyRuntime);
393393
}
394394

395+
static void
396+
queue_untracked_obj_decref(PyObject *op, struct collection_state *state)
397+
{
398+
if (!_PyObject_GC_IS_TRACKED(op)) {
399+
// GC objects with zero refcount are handled subsequently by the
400+
// GC as if they were cyclic trash, but we have to handle dead
401+
// non-GC objects here. Add one to the refcount so that we can
402+
// decref and deallocate the object once we start the world again.
403+
op->ob_ref_shared += (1 << _Py_REF_SHARED_SHIFT);
404+
#ifdef Py_REF_DEBUG
405+
_Py_IncRefTotal(_PyThreadState_GET());
406+
#endif
407+
worklist_push(&state->objs_to_decref, op);
408+
}
409+
410+
}
411+
395412
static void
396413
merge_queued_objects(_PyThreadStateImpl *tstate, struct collection_state *state)
397414
{
@@ -403,36 +420,16 @@ merge_queued_objects(_PyThreadStateImpl *tstate, struct collection_state *state)
403420
// Subtract one when merging because the queue had a reference.
404421
Py_ssize_t refcount = merge_refcount(op, -1);
405422

406-
if (!_PyObject_GC_IS_TRACKED(op) && refcount == 0) {
407-
// GC objects with zero refcount are handled subsequently by the
408-
// GC as if they were cyclic trash, but we have to handle dead
409-
// non-GC objects here. Add one to the refcount so that we can
410-
// decref and deallocate the object once we start the world again.
411-
op->ob_ref_shared += (1 << _Py_REF_SHARED_SHIFT);
412-
#ifdef Py_REF_DEBUG
413-
_Py_IncRefTotal(_PyThreadState_GET());
414-
#endif
415-
worklist_push(&state->objs_to_decref, op);
423+
if (refcount == 0) {
424+
queue_untracked_obj_decref(op, state);
416425
}
417426
}
418427
}
419428

420429
static void
421430
queue_freed_object(PyObject *obj, void *arg)
422431
{
423-
struct collection_state *state = (struct collection_state *)arg;
424-
425-
// GC objects with zero refcount are handled subsequently by the
426-
// GC as if they were cyclic trash, but we have to handle dead
427-
// non-GC objects here. Add one to the refcount so that we can
428-
// decref and deallocate the object once we start the world again.
429-
if (!_PyObject_GC_IS_TRACKED(obj)) {
430-
obj->ob_ref_shared += (1 << _Py_REF_SHARED_SHIFT);
431-
#ifdef Py_REF_DEBUG
432-
_Py_IncRefTotal(_PyThreadState_GET());
433-
#endif
434-
worklist_push(&state->objs_to_decref, obj);
435-
}
432+
queue_untracked_obj_decref(obj, arg);
436433
}
437434

438435
static void

0 commit comments

Comments
 (0)