Address Guido's and Yury's reviews.

Author: picnixz

Misc/NEWS.d/next/Library/2024-10-22-12-47-46.gh-issue-125789.Hk885p.rst

@@ -1,3 +1,3 @@
-Mitigate interpreter's crashes and state corruption due to side-effects in
-:meth:`asyncio.Future.remove_done_callback` or others callback scheduling
+Mitigate interpreter crashes and state corruption due to side-effects in
+:meth:`asyncio.Future.remove_done_callback` or other callback scheduling
 methods. Patch by Bénédikt Tran.

Modules/_asynciomodule.c

@@ -434,8 +434,8 @@ future_schedule_callbacks(asyncio_state *state, FutureObj *fut)
         return 0;
     }
 
-    if (!PyList_Check(fut->fut_callbacks)) {
-        PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
+    if (!PyList_CheckExact(fut->fut_callbacks)) {
+        PyErr_SetString(PyExc_RuntimeError, "corrupted callbacks list");
         return -1;
     }
 
@@ -446,19 +446,19 @@ future_schedule_callbacks(asyncio_state *state, FutureObj *fut)
         return 0;
     }
 
-    // Beware: 'call_soon' below may change fut_callbacks or its items
+    // Beware: An evil 'call_soon' could change fut_callbacks or its items
     // (see https://github.com/python/cpython/issues/125789 for details).
     for (i = 0; fut->fut_callbacks != NULL; i++) {
-        if (!PyList_Check(fut->fut_callbacks)) {
-            PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
+        if (!PyList_CheckExact(fut->fut_callbacks)) {
+            PyErr_SetString(PyExc_RuntimeError, "corrupted callbacks list");
             return -1;
         }
         if (i >= PyList_GET_SIZE(fut->fut_callbacks)) {
             break; // done
         }
         PyObject *cb_tup = PyList_GET_ITEM(fut->fut_callbacks, i);
-        if (!PyTuple_Check(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 2) {
-            PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
+        if (!PyTuple_CheckExact(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 2) {
+            PyErr_SetString(PyExc_RuntimeError, "corrupted callback tuple");
             return -1;
         }
         PyObject *cb = PyTuple_GET_ITEM(cb_tup, 0);
@@ -1035,6 +1035,8 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
     ENSURE_FUTURE_ALIVE(state, self)
 
     if (self->fut_callback0 != NULL) {
+        // Beware: An evil PyObject_RichCompareBool could change fut_callback0
+        // (see https://github.com/python/cpython/issues/125789 for details).
         int cmp = PyObject_RichCompareBool(self->fut_callback0, fn, Py_EQ);
         if (cmp == -1) {
             return NULL;
@@ -1051,8 +1053,8 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
         return PyLong_FromSsize_t(cleared_callback0);
     }
 
-    if (!PyList_Check(self->fut_callbacks)) {
-        PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
+    if (!PyList_CheckExact(self->fut_callbacks)) {
+        PyErr_SetString(PyExc_RuntimeError, "corrupted callbacks list");
         return NULL;
     }
 
@@ -1064,11 +1066,11 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
 
     if (len == 1) {
         PyObject *cb_tup = PyList_GET_ITEM(self->fut_callbacks, 0);
-        // Beware: PyObject_RichCompareBool below may change fut_callbacks or
-        // its items (see https://github.com/python/cpython/issues/97592 and
+        // Beware: An evil PyObject_RichCompareBool could change fut_callbacks
+        // or its items (see https://github.com/python/cpython/issues/97592 or
         // https://github.com/python/cpython/issues/125789 for details).
-        if (!PyTuple_Check(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 1) {
-            PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
+        if (!PyTuple_CheckExact(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 1) {
+            PyErr_SetString(PyExc_RuntimeError, "corrupted callback tuple");
             return NULL;
         }
         Py_INCREF(cb_tup);
@@ -1092,20 +1094,20 @@ _asyncio_Future_remove_done_callback_impl(FutureObj *self, PyTypeObject *cls,
         return NULL;
     }
 
-    // Beware: PyObject_RichCompareBool below may change fut_callbacks or
-    // its items (see https://github.com/python/cpython/issues/97592 and
+    // Beware: An evil PyObject_RichCompareBool could change fut_callbacks
+    // or its items (see https://github.com/python/cpython/issues/97592 or
     // https://github.com/python/cpython/issues/125789 for details).
     for (i = 0; self->fut_callbacks != NULL; i++) {
-        if (!PyList_Check(self->fut_callbacks)) {
-            PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
+        if (!PyList_CheckExact(self->fut_callbacks)) {
+            PyErr_SetString(PyExc_RuntimeError, "corrupted callbacks list");
             goto fail;
         }
         if (i >= PyList_GET_SIZE(self->fut_callbacks)) {
             break; // done
         }
         PyObject *cb_tup = PyList_GET_ITEM(self->fut_callbacks, i);
-        if (!PyTuple_Check(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 1) {
-            PyErr_SetString(PyExc_RuntimeError, "corrupted future state");
+        if (!PyTuple_CheckExact(cb_tup) || PyTuple_GET_SIZE(cb_tup) < 1) {
+            PyErr_SetString(PyExc_RuntimeError, "corrupted callback tuple");
             goto fail;
         }
         Py_INCREF(cb_tup);