]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
[3.13] gh-120974: Make _asyncio._leave_task atomic in the free-threaded build (GH...
authorMiss Islington (bot) <31488909+miss-islington@users.noreply.github.com>
Tue, 23 Jul 2024 17:30:58 +0000 (19:30 +0200)
committerGitHub <noreply@github.com>
Tue, 23 Jul 2024 17:30:58 +0000 (17:30 +0000)
gh-120974: Make _asyncio._leave_task atomic in the free-threaded build (GH-122139)

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

Update `_PyDict_DelItemIf` to allow for an argument to be passed to the
predicate.
(cherry picked from commit a15feded71dd47202db169613effdafc468a8cf3)

Co-authored-by: Sam Gross <colesbury@gmail.com>
Include/internal/pycore_dict.h
Modules/_asynciomodule.c
Modules/_weakref.c
Objects/dictobject.c

index 01f80e21ba2797fa73bf6f83cc4fc46dff2064dc..1c4a63c2271a9353a5aa37fa28d397b5fab94386 100644 (file)
@@ -16,8 +16,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
index fffa6d25d022acf96d3154bbcc20ff083280a16c..429322c2bf177b1c5015d8462f5b4f98412d7989 100644 (file)
@@ -1945,30 +1945,36 @@ enter_task(asyncio_state *state, PyObject *loop, PyObject *task)
     return 0;
 }
 
+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 *
index a5c15c0f10b930a7d8fcb71349a6523bb623fefa..ecaa08ff60f2034dbf7c60cf10d129ce0ee0d5b4 100644 (file)
@@ -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");
@@ -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;
 }
index 431c4965bdec094086ad780ff2eddbfb06b7ba8e..e9861d471369317281e37958f90c2b014700805d 100644 (file)
@@ -2567,7 +2567,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)
 {
@@ -2609,7 +2609,6 @@ delitem_common(PyDictObject *mp, Py_hash_t hash, Py_ssize_t ix,
     Py_DECREF(old_value);
 
     ASSERT_CONSISTENT(mp);
-    return 0;
 }
 
 int
@@ -2652,7 +2651,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
@@ -2667,7 +2667,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;
@@ -2677,24 +2678,20 @@ 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;
 
@@ -2702,7 +2699,8 @@ delitemif_lock_held(PyObject *op, PyObject *key,
         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;
     }
@@ -2714,11 +2712,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;
 }