]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
[3.12] GH-125789: fix `fut._callbacks` to always return a copy of callbacks (GH-12592...
authorMiss Islington (bot) <31488909+miss-islington@users.noreply.github.com>
Fri, 25 Oct 2024 14:02:46 +0000 (16:02 +0200)
committerGitHub <noreply@github.com>
Fri, 25 Oct 2024 14:02:46 +0000 (19:32 +0530)
GH-125789: fix `fut._callbacks` to always return a copy of callbacks (GH-125922)

Fix `asyncio.Future._callbacks` to always return a copy of the internal list of callbacks to avoid mutation from user code affecting the internal state.

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
(cherry picked from commit cae853e3b44cd5cb033b904e163c490dd28bc30a)

Lib/test/test_asyncio/test_futures.py
Misc/NEWS.d/next/Library/2024-10-24-14-08-10.gh-issue-125789.eaiAMw.rst [new file with mode: 0644]
Modules/_asynciomodule.c

index 050d33f4fab3ed7e430e27388a759e18afd77045..60e93a268ab00df821476c84db2fb44bc14eacb5 100644 (file)
@@ -686,6 +686,24 @@ class CFutureTests(BaseFutureTests, test_utils.TestCase):
         evil = gc.get_referents(_asyncio)
         gc.collect()
 
+    def test_callbacks_copy(self):
+        # See https://github.com/python/cpython/issues/125789
+        # In C implementation, the `_callbacks` attribute
+        # always returns a new list to avoid mutations of internal state
+
+        fut = self._new_future(loop=self.loop)
+        f1 = lambda _: 1
+        f2 = lambda _: 2
+        fut.add_done_callback(f1)
+        fut.add_done_callback(f2)
+        callbacks = fut._callbacks
+        self.assertIsNot(callbacks, fut._callbacks)
+        fut.remove_done_callback(f1)
+        callbacks = fut._callbacks
+        self.assertIsNot(callbacks, fut._callbacks)
+        fut.remove_done_callback(f2)
+        self.assertIsNone(fut._callbacks)
+
 
 @unittest.skipUnless(hasattr(futures, '_CFuture'),
                      'requires the C _asyncio module')
diff --git a/Misc/NEWS.d/next/Library/2024-10-24-14-08-10.gh-issue-125789.eaiAMw.rst b/Misc/NEWS.d/next/Library/2024-10-24-14-08-10.gh-issue-125789.eaiAMw.rst
new file mode 100644 (file)
index 0000000..964a006
--- /dev/null
@@ -0,0 +1 @@
+Fix possible crash when mutating list of callbacks returned by :attr:`!asyncio.Future._callbacks`. It now always returns a new copy in C implementation :mod:`!_asyncio`. Patch by Kumar Aditya.
index da44bb6b71457870915152e0a65397ff87f89f9c..bd4eaf083f7fed4bed9b8b9121610e7b7d4a9d1a 100644 (file)
@@ -1290,52 +1290,49 @@ static PyObject *
 FutureObj_get_callbacks(FutureObj *fut, void *Py_UNUSED(ignored))
 {
     asyncio_state *state = get_asyncio_state_by_def((PyObject *)fut);
-    Py_ssize_t i;
-
     ENSURE_FUTURE_ALIVE(state, fut)
 
-    if (fut->fut_callback0 == NULL) {
-        if (fut->fut_callbacks == NULL) {
-            Py_RETURN_NONE;
-        }
-
-        return Py_NewRef(fut->fut_callbacks);
+    Py_ssize_t len = 0;
+    if (fut->fut_callback0 != NULL) {
+        len++;
     }
-
-    Py_ssize_t len = 1;
     if (fut->fut_callbacks != NULL) {
         len += PyList_GET_SIZE(fut->fut_callbacks);
     }
 
-
-    PyObject *new_list = PyList_New(len);
-    if (new_list == NULL) {
-        return NULL;
+    if (len == 0) {
+        Py_RETURN_NONE;
     }
 
-    PyObject *tup0 = PyTuple_New(2);
-    if (tup0 == NULL) {
-        Py_DECREF(new_list);
+    PyObject *callbacks = PyList_New(len);
+    if (callbacks == NULL) {
         return NULL;
     }
 
-    Py_INCREF(fut->fut_callback0);
-    PyTuple_SET_ITEM(tup0, 0, fut->fut_callback0);
-    assert(fut->fut_context0 != NULL);
-    Py_INCREF(fut->fut_context0);
-    PyTuple_SET_ITEM(tup0, 1, (PyObject *)fut->fut_context0);
-
-    PyList_SET_ITEM(new_list, 0, tup0);
+    Py_ssize_t i = 0;
+    if (fut->fut_callback0 != NULL) {
+        PyObject *tup0 = PyTuple_New(2);
+        if (tup0 == NULL) {
+            Py_DECREF(callbacks);
+            return NULL;
+        }
+        PyTuple_SET_ITEM(tup0, 0, Py_NewRef(fut->fut_callback0));
+        assert(fut->fut_context0 != NULL);
+        PyTuple_SET_ITEM(tup0, 1, Py_NewRef(fut->fut_context0));
+        PyList_SET_ITEM(callbacks, i, tup0);
+        i++;
+    }
 
     if (fut->fut_callbacks != NULL) {
-        for (i = 0; i < PyList_GET_SIZE(fut->fut_callbacks); i++) {
-            PyObject *cb = PyList_GET_ITEM(fut->fut_callbacks, i);
+        for (Py_ssize_t j = 0; j < PyList_GET_SIZE(fut->fut_callbacks); j++) {
+            PyObject *cb = PyList_GET_ITEM(fut->fut_callbacks, j);
             Py_INCREF(cb);
-            PyList_SET_ITEM(new_list, i + 1, cb);
+            PyList_SET_ITEM(callbacks, i, cb);
+            i++;
         }
     }
 
-    return new_list;
+    return callbacks;
 }
 
 static PyObject *