From a6598575f4bc20f9a01c2bced2d0b1ff14d7576f Mon Sep 17 00:00:00 2001 From: Ralf Grosse-Kunstleve Date: Tue, 11 Jan 2022 19:34:38 +0100 Subject: [LLDB] Fix Python GIL-not-held issues The GIL must be held when calling any Python C API functions. In multithreaded applications that use callbacks this requirement can easily be violated by accident. A general tool to ensure GIL health is not available, but patching Python Py_INCREF to add an assert provides a basic health check: ``` +int PyGILState_Check(void); /* Include/internal/pystate.h */ + #define Py_INCREF(op) ( \ + assert(PyGILState_Check()), \ _Py_INC_REFTOTAL _Py_REF_DEBUG_COMMA \ ((PyObject *)(op))->ob_refcnt++) #define Py_DECREF(op) \ do { \ + assert(PyGILState_Check()); \ PyObject *_py_decref_tmp = (PyObject *)(op); \ if (_Py_DEC_REFTOTAL _Py_REF_DEBUG_COMMA \ --(_py_decref_tmp)->ob_refcnt != 0) \ ``` Adding this assertion causes around 50 test failures in LLDB. Adjusting the scope of things guarded by `py_lock` fixes them. More background: https://docs.python.org/3/glossary.html#term-global-interpreter-lock Patch by Ralf Grosse-Kunstleve Differential Revision: https://reviews.llvm.org/D114722 --- .../ScriptInterpreter/Python/PythonDataObjects.h | 25 ++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) (limited to 'lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h') diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h index 56bc55d..9d2cdca 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h @@ -88,12 +88,21 @@ public: StructuredPythonObject() : StructuredData::Generic() {} StructuredPythonObject(void *obj) : StructuredData::Generic(obj) { + assert(PyGILState_Check()); Py_XINCREF(GetValue()); } ~StructuredPythonObject() override { - if (Py_IsInitialized()) - Py_XDECREF(GetValue()); + if (Py_IsInitialized()) { + if (_Py_IsFinalizing()) { + // Leak GetValue() rather than crashing the process. + // https://docs.python.org/3/c-api/init.html#c.PyGILState_Ensure + } else { + PyGILState_STATE state = PyGILState_Ensure(); + Py_XDECREF(GetValue()); + PyGILState_Release(state); + } + } SetValue(nullptr); } @@ -264,8 +273,16 @@ public: ~PythonObject() { Reset(); } void Reset() { - if (m_py_obj && Py_IsInitialized()) - Py_DECREF(m_py_obj); + if (m_py_obj && Py_IsInitialized()) { + if (_Py_IsFinalizing()) { + // Leak m_py_obj rather than crashing the process. + // https://docs.python.org/3/c-api/init.html#c.PyGILState_Ensure + } else { + PyGILState_STATE state = PyGILState_Ensure(); + Py_DECREF(m_py_obj); + PyGILState_Release(state); + } + } m_py_obj = nullptr; } -- cgit v1.1