segfault in 'async def' coroutines
Hi, Under some circumstances, in asyncio code that runs in uvloop [1], cython code segfaults in cython/Cython/Utility/Coroutine.c: static PyObject * __Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { Py_INCREF(self->gi_qualname); return self->gi_qualname; } "self->gi_qualname" can be NULL. The correct code is probably: __Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { if (self->gi_qualname == NULL) { return __pyx_empty_unicode; } Py_INCREF(self->gi_qualname); return self->gi_qualname; } Thanks, Yury
I've just discovered that same thing has to be fixed for __name__. On 2016-05-14 5:31 PM, Yury Selivanov wrote:
Hi,
Under some circumstances, in asyncio code that runs in uvloop [1], cython code segfaults in cython/Cython/Utility/Coroutine.c:
static PyObject * __Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { Py_INCREF(self->gi_qualname); return self->gi_qualname; }
"self->gi_qualname" can be NULL. The correct code is probably:
__Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { if (self->gi_qualname == NULL) { return __pyx_empty_unicode; } Py_INCREF(self->gi_qualname); return self->gi_qualname; }
Thanks, Yury
Yury Selivanov schrieb am 14.05.2016 um 23:31:
Under some circumstances, in asyncio code that runs in uvloop [1], cython code segfaults in cython/Cython/Utility/Coroutine.c:
static PyObject * __Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { Py_INCREF(self->gi_qualname); return self->gi_qualname; }
"self->gi_qualname" can be NULL. The correct code is probably:
__Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { if (self->gi_qualname == NULL) { return __pyx_empty_unicode; } Py_INCREF(self->gi_qualname); return self->gi_qualname; }
I wonder why it can be NULL. It's supposed to be set to a string constant at creation time. See GeneratorDefNode in Nodes.py. Is there anything special you are doing with these objects? Could you try to figure out how the ones that have a NULL value here are created? Stefan
On 2016-05-16 3:58 AM, Stefan Behnel wrote:
Under some circumstances, in asyncio code that runs in uvloop [1], cython code segfaults in cython/Cython/Utility/Coroutine.c:
static PyObject * __Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { Py_INCREF(self->gi_qualname); return self->gi_qualname; }
"self->gi_qualname" can be NULL. The correct code is probably:
__Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { if (self->gi_qualname == NULL) { return __pyx_empty_unicode; } Py_INCREF(self->gi_qualname); return self->gi_qualname; } I wonder why it can be NULL. It's supposed to be set to a string constant at creation time. See GeneratorDefNode in Nodes.py. Is there anything special you are doing with these objects? Could you try to figure out how
Yury Selivanov schrieb am 14.05.2016 um 23:31: the ones that have a NULL value here are created?
It's probably because asyncio Future and Task have __del__ methods which (may) try to call repr on coroutines. At that point of time, maybe the GC breaks the refs. Would you be able to fix the refs for qualname/name? Yury
Yury Selivanov schrieb am 16.05.2016 um 22:19:
On 2016-05-16 3:58 AM, Stefan Behnel wrote:
Under some circumstances, in asyncio code that runs in uvloop [1], cython code segfaults in cython/Cython/Utility/Coroutine.c:
static PyObject * __Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { Py_INCREF(self->gi_qualname); return self->gi_qualname; }
"self->gi_qualname" can be NULL. The correct code is probably:
__Pyx_Coroutine_get_qualname(__pyx_CoroutineObject *self) { if (self->gi_qualname == NULL) { return __pyx_empty_unicode; } Py_INCREF(self->gi_qualname); return self->gi_qualname; } I wonder why it can be NULL. It's supposed to be set to a string constant at creation time. See GeneratorDefNode in Nodes.py. Is there anything special you are doing with these objects? Could you try to figure out how
Yury Selivanov schrieb am 14.05.2016 um 23:31: the ones that have a NULL value here are created?
It's probably because asyncio Future and Task have __del__ methods which (may) try to call repr on coroutines. At that point of time, maybe the GC breaks the refs.
Would you be able to fix the refs for qualname/name?
It seems that CPython should suffer from the same problem, though. It uses the same code in the getter. Also, should we really return an empty string or None? Both feel like unexpected results, so I can't tell which is better. And finally, since both name values are guaranteed to be strings (the setter checks their type), I wonder if we shouldn't just make sure they are *exactly* Unicode strings by converting any subtypes, and then remove their Py_CLEAR() from the tp_clear() function. Strings can't participate in reference cycles, and we could thus report the actual names even during garbage collection. Stefan
On 2016-05-17 1:32 AM, Stefan Behnel wrote:
And finally, since both name values are guaranteed to be strings (the setter checks their type), I wonder if we shouldn't just make sure they are *exactly* Unicode strings by converting any subtypes, and then remove their Py_CLEAR() from the tp_clear() function. Strings can't participate in reference cycles, and we could thus report the actual names even during garbage collection.
This sounds like a really nice solution, Stefan. Yury
Stefan, any ETA on this? For now I have to patch the C file generated by Cython (otherwise uvloop segfaults), and that's kind of fragile. Yury On 2016-05-17 11:31 AM, Yury Selivanov wrote:
On 2016-05-17 1:32 AM, Stefan Behnel wrote:
And finally, since both name values are guaranteed to be strings (the setter checks their type), I wonder if we shouldn't just make sure they are *exactly* Unicode strings by converting any subtypes, and then remove their Py_CLEAR() from the tp_clear() function. Strings can't participate in reference cycles, and we could thus report the actual names even during garbage collection.
This sounds like a really nice solution, Stefan.
Yury
Yury Selivanov schrieb am 19.05.2016 um 17:29:
On 2016-05-17 11:31 AM, Yury Selivanov wrote:
On 2016-05-17 1:32 AM, Stefan Behnel wrote:
And finally, since both name values are guaranteed to be strings (the setter checks their type), I wonder if we shouldn't just make sure they are *exactly* Unicode strings by converting any subtypes, and then remove their Py_CLEAR() from the tp_clear() function. Strings can't participate in reference cycles, and we could thus report the actual names even during garbage collection.
This sounds like a really nice solution, Stefan.
Stefan, any ETA on this? For now I have to patch the C file generated by Cython (otherwise uvloop segfaults), and that's kind of fragile.
Could you give it a try yourself? The code is in Coroutine.c, should be straight forward. Stefan
participants (2)
-
Stefan Behnel -
Yury Selivanov