Skip to content

gh-120974: Make _asyncio._leave_task atomic in the free-threaded build #122139

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 3 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
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
8 changes: 6 additions & 2 deletions Include/internal/pycore_dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ extern "C" {
// Unsafe flavor of PyDict_GetItemWithError(): no error checking
extern PyObject* _PyDict_GetItemWithError(PyObject *dp, PyObject *key);

extern int _PyDict_DelItemIf(PyObject *mp, PyObject *key,
int (*predicate)(PyObject *value));
// Delete an item from a dict if a predicate is true
// Returns -1 on error, 1 if the item was deleted, 0 otherwise
// Export for '_asyncio' shared extension
PyAPI_FUNC(int) _PyDict_DelItemIf(PyObject *mp, PyObject *key,
int (*predicate)(PyObject *value, void *arg),
void *arg);

// "KnownHash" variants
// Export for '_asyncio' shared extension
Expand Down
42 changes: 24 additions & 18 deletions Modules/_asynciomodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2031,30 +2031,36 @@ enter_task(asyncio_state *state, PyObject *loop, PyObject *task)
return _PyDict_SetItem_KnownHash(state->current_tasks, loop, task, hash);
}

static int
err_leave_task(PyObject *item, PyObject *task)
{
PyErr_Format(
PyExc_RuntimeError,
"Leaving task %R does not match the current task %R.",
task, item);
return -1;
}

static int
leave_task_predicate(PyObject *item, void *task)
{
if (item != task) {
return err_leave_task(item, (PyObject *)task);
}
return 1;
}

static int
leave_task(asyncio_state *state, PyObject *loop, PyObject *task)
/*[clinic end generated code: output=0ebf6db4b858fb41 input=51296a46313d1ad8]*/
{
PyObject *item;
Py_hash_t hash;
hash = PyObject_Hash(loop);
if (hash == -1) {
return -1;
}
item = _PyDict_GetItem_KnownHash(state->current_tasks, loop, hash);
if (item != task) {
if (item == NULL) {
/* Not entered, replace with None */
item = Py_None;
}
PyErr_Format(
PyExc_RuntimeError,
"Leaving task %R does not match the current task %R.",
task, item, NULL);
return -1;
int res = _PyDict_DelItemIf(state->current_tasks, loop,
leave_task_predicate, task);
if (res == 0) {
// task was not found
return err_leave_task(Py_None, task);
}
return _PyDict_DelItem_KnownHash(state->current_tasks, loop, hash);
return res;
}

static PyObject *
Expand Down
13 changes: 3 additions & 10 deletions Modules/_weakref.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ _weakref_getweakrefcount_impl(PyObject *module, PyObject *object)


static int
is_dead_weakref(PyObject *value)
is_dead_weakref(PyObject *value, void *unused)
{
if (!PyWeakref_Check(value)) {
PyErr_SetString(PyExc_TypeError, "not a weakref");
Expand All @@ -56,15 +56,8 @@ _weakref__remove_dead_weakref_impl(PyObject *module, PyObject *dct,
PyObject *key)
/*[clinic end generated code: output=d9ff53061fcb875c input=19fc91f257f96a1d]*/
{
if (_PyDict_DelItemIf(dct, key, is_dead_weakref) < 0) {
if (PyErr_ExceptionMatches(PyExc_KeyError))
/* This function is meant to allow safe weak-value dicts
with GC in another thread (see issue #28427), so it's
ok if the key doesn't exist anymore.
*/
PyErr_Clear();
else
return NULL;
if (_PyDict_DelItemIf(dct, key, is_dead_weakref, NULL) < 0) {
return NULL;
}
Py_RETURN_NONE;
}
Expand Down
30 changes: 15 additions & 15 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2508,7 +2508,7 @@ delete_index_from_values(PyDictValues *values, Py_ssize_t ix)
values->size = size;
}

static int
static void
delitem_common(PyDictObject *mp, Py_hash_t hash, Py_ssize_t ix,
PyObject *old_value, uint64_t new_version)
{
Expand Down Expand Up @@ -2550,7 +2550,6 @@ delitem_common(PyDictObject *mp, Py_hash_t hash, Py_ssize_t ix,
Py_DECREF(old_value);

ASSERT_CONSISTENT(mp);
return 0;
}

int
Expand Down Expand Up @@ -2593,7 +2592,8 @@ delitem_knownhash_lock_held(PyObject *op, PyObject *key, Py_hash_t hash)
PyInterpreterState *interp = _PyInterpreterState_GET();
uint64_t new_version = _PyDict_NotifyEvent(
interp, PyDict_EVENT_DELETED, mp, key, NULL);
return delitem_common(mp, hash, ix, old_value, new_version);
delitem_common(mp, hash, ix, old_value, new_version);
return 0;
}

int
Expand All @@ -2608,7 +2608,8 @@ _PyDict_DelItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)

static int
delitemif_lock_held(PyObject *op, PyObject *key,
int (*predicate)(PyObject *value))
int (*predicate)(PyObject *value, void *arg),
void *arg)
{
Py_ssize_t ix;
PyDictObject *mp;
Expand All @@ -2618,32 +2619,29 @@ delitemif_lock_held(PyObject *op, PyObject *key,

ASSERT_DICT_LOCKED(op);

if (!PyDict_Check(op)) {
PyErr_BadInternalCall();
return -1;
}
assert(key);
hash = PyObject_Hash(key);
if (hash == -1)
return -1;
mp = (PyDictObject *)op;
ix = _Py_dict_lookup(mp, key, hash, &old_value);
if (ix == DKIX_ERROR)
if (ix == DKIX_ERROR) {
return -1;
}
if (ix == DKIX_EMPTY || old_value == NULL) {
_PyErr_SetKeyError(key);
return -1;
return 0;
}

res = predicate(old_value);
res = predicate(old_value, arg);
if (res == -1)
return -1;

if (res > 0) {
PyInterpreterState *interp = _PyInterpreterState_GET();
uint64_t new_version = _PyDict_NotifyEvent(
interp, PyDict_EVENT_DELETED, mp, key, NULL);
return delitem_common(mp, hash, ix, old_value, new_version);
delitem_common(mp, hash, ix, old_value, new_version);
return 1;
} else {
return 0;
}
Expand All @@ -2655,11 +2653,13 @@ delitemif_lock_held(PyObject *op, PyObject *key,
*/
int
_PyDict_DelItemIf(PyObject *op, PyObject *key,
int (*predicate)(PyObject *value))
int (*predicate)(PyObject *value, void *arg),
void *arg)
{
assert(PyDict_Check(op));
int res;
Py_BEGIN_CRITICAL_SECTION(op);
res = delitemif_lock_held(op, key, predicate);
res = delitemif_lock_held(op, key, predicate, arg);
Py_END_CRITICAL_SECTION();
return res;
}
Expand Down
Loading