python-greenlet / greenlet

Lightweight in-process concurrent programming
Other
1.63k stars 247 forks source link

Regression: GREENLET_BROKEN_PY_ADD_PENDING is never set for greenlet 3.0.0 #376

Closed godlygeek closed 10 months ago

godlygeek commented 10 months ago

https://github.com/python-greenlet/greenlet/commit/215d0c98dcbefd68357c90e86c526b00026171c8.patch factored the code that defines GREENLET_BROKEN_PY_ADD_PENDING into a new file, greenlet_cpython_add_pending.hpp, but that file is never #include'd, so the workaround for Py_AddPendingCall failing during interpreter finalization on 3.8 is never used.

I found this while debugging a segfault:

Thread 1 (Thread 0x7fb303fbf700 (LWP 141819) (Exiting)):
#0  0x00007fb32a338ae1 in _PyErr_Fetch (p_traceback=<synthetic pointer>, p_value=<synthetic pointer>, p_type=<synthetic pointer>, tstate=0x0) at src/python/python3.8/Python/errors.c:399
#1  _PyEval_AddPendingCall (tstate=0x0, ceval=0x7fb32a58f2a8 <_PyRuntime+584>, func=0x7fb3220d1b40 <greenlet::ThreadState_DestroyNoGIL::DestroyQueueWithGIL(void*)>, arg=0x0) at src/python/python3.8/Python/ceval.c:486
#2  0x00007fb3220d295e in greenlet::ThreadState_DestroyNoGIL::AddPendingCall (arg=0x0, func=0x7fb3220d1b40 <greenlet::ThreadState_DestroyNoGIL::DestroyQueueWithGIL(void*)>) at src/greenlet/TThreadStateDestroy.cpp:94
#3  greenlet::ThreadState_DestroyNoGIL::ThreadState_DestroyNoGIL (state=0x7fb303fd3510, this=<synthetic pointer>) at src/greenlet/TThreadStateDestroy.cpp:142
#4  greenlet::ThreadStateCreator<greenlet::ThreadState_DestroyNoGIL>::~ThreadStateCreator (this=<optimized out>, __in_chrg=<optimized out>) at src/greenlet/greenlet_thread_state.hpp:488
#5  0x00007fb321e07dd9 in ?? () from /lib64/libstdc++.so.6
#6  0x00007fb329de9ca2 in __nptl_deallocate_tsd () from /lib64/libpthread.so.0
#7  0x00007fb329de9eb3 in start_thread () from /lib64/libpthread.so.0
#8  0x00007fb329409b0d in clone () from /lib64/libc.so.6

Note that src/greenlet/TThreadStateDestroy.cpp:94 should never be used on Python 3.8: https://github.com/python-greenlet/greenlet/blob/660a66f339c61b2572ea6237cc042bde356419b1/src/greenlet/TThreadStateDestroy.cpp#L91-L95

But it winds up being used because there's no #include <greenlet_cpython_add_pending.hpp>

jamadden commented 10 months ago

Thanks! It'll be a few days until I can deal with this, I'm coping with the second death in close family in the last two weeks.