Bugs in asyncio.Future.remove_done_callback() cause segfault.

[xiaxinmeng]

Crash report

The following example triggers a segfault on the latest stable Python3.8.14. I think there might be a bug in asyncio.Future.remove_done_callback().

import asyncio
fut = asyncio.Future()
fut.add_done_callback(str)
for str in range(63):
 fut.add_done_callback(id)

class evil:
 def __eq__(self, other):
 fut.remove_done_callback(other)
fut.remove_done_callback(evil())

Error messages

Segmentation fault (core dumped)

Your environment

• CPython versions tested on: Python 3.8.14, Python 3.9.0, main branch 68c46ae
• Operating system and architecture: [GCC 7.5.0] on linux

[gvanrossum]

Yeah, I can still repro this on 3.11. It doesn't crash when the pure-Python asyncio implementation is used (sys.modules['_asyncio'] = None) so this appears to be crashing in _asyncio.

Someone needs to look into the code there and see what is happening.

[CharlieZhao95]

I simply debugged with the following code:

import asyncio

async def crash():
 fut = asyncio.Future()
 fut.add_done_callback(str)
 for _ in range(10):
 fut.add_done_callback(id)

 class Evil:
 def __eq__(self, other):
 fut.remove_done_callback(other)

 fut.remove_done_callback(Evil())
 asyncio.create_task(set_after(fut, 1, '... world'))
 print('hello ...')
 print(await fut)


if __name__ == "__main__":
 asyncio.run(crash())

The core dump seems to happen in the _asyncio_Future_remove_done_callback function of _asynciomodule.c.
At some point, the self->fut_callbacks is set to null, which causes crashing. In my tests it seems to be related to Py_CLEAR(self->fut_callbacks);

static PyObject *
_asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn)
{
...
 // The `self->fut_callbacks` here is a nullptr, which causes the core dump
 for (i = 0; i < PyList_GET_SIZE(self->fut_callbacks); i++){
 int ret;
 PyObject *item = PyList_GET_ITEM(self->fut_callbacks, i);
 Py_INCREF(item);
 ret = PyObject_RichCompareBool(PyTuple_GET_ITEM(item, 0), fn, Py_EQ);
 if (ret == 0){
 if (j < len){
 PyList_SET_ITEM(newlist, j, item);
 j++;
 continue;
 }
 ret = PyList_Append(newlist, item);
 }
 Py_DECREF(item);
 if (ret < 0){
 goto fail;
 }
 }

 if (j == 0){
 Py_CLEAR(self->fut_callbacks);
 Py_DECREF(newlist);
 return PyLong_FromSsize_t(len + cleared_callback0);
 }
}

[gvanrossum]

Thanks, that's good research! The cause is now clear: the PyObject_RichCompareBool call invokes the evil class's __eq__ method, which calls the same function and ends up clearing fut_callbacks.

The fix would seem to require some check for whether fut_callbacks is NULL before doing anything further to it. This seems a little tricky, we need to check in the for loop header, but also in the code (that you snipped) below that assigns newlist to a slice of fut_callbacks.

Do you want to create the PR to get full credit?

[CharlieZhao95]

Thanks for your guidance! I notice that you have created a commit, I wonder if it would be better to add a test case that can trigger the crash :)