maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Ralf Grosse-Kunstleve 2022-01-11 19:34:38 +01:00 committed by Pavel Labath
parent af12a3f4a9
commit a6598575f4
3 changed files with 57 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
View File

@ -257,6 +257,7 @@ PythonObject PythonObject::GetAttributeValue(llvm::StringRef attr) const {
}
StructuredData::ObjectSP PythonObject::CreateStructuredObject() const {
assert(PyGILState_Check());
switch (GetObjectType()) {
case PyObjectType::Dictionary:
return PythonDictionary(PyRefType::Borrowed, m_py_obj)

25
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
View File

@ -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;
}

97
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
View File

@ -1438,14 +1438,9 @@ ScriptInterpreterPythonImpl::CreateFrameRecognizer(const char *class_name) {
if (class_name == nullptr || class_name[0] == '\0')
return StructuredData::GenericSP();
void *ret_val;
{
Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN,
Locker::FreeLock);
ret_val = LLDBSWIGPython_CreateFrameRecognizer(class_name,
m_dictionary_name.c_str());
}
Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
void *ret_val = LLDBSWIGPython_CreateFrameRecognizer(
class_name, m_dictionary_name.c_str());
return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
}
@ -1502,14 +1497,9 @@ ScriptInterpreterPythonImpl::OSPlugin_CreatePluginObject(
if (!process_sp)
return StructuredData::GenericSP();
void *ret_val;
{
Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN,
Locker::FreeLock);
ret_val = LLDBSWIGPythonCreateOSPlugin(
class_name, m_dictionary_name.c_str(), process_sp);
}
Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
void *ret_val = LLDBSWIGPythonCreateOSPlugin(
class_name, m_dictionary_name.c_str(), process_sp);
return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
}
@ -1757,17 +1747,13 @@ StructuredData::ObjectSP ScriptInterpreterPythonImpl::CreateScriptedThreadPlan(
if (!python_interpreter)
return {};
void *ret_val;
{
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
ret_val = LLDBSwigPythonCreateScriptedThreadPlan(
class_name, python_interpreter->m_dictionary_name.c_str(),
args_data, error_str, thread_plan_sp);
if (!ret_val)
return {};
}
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
void *ret_val = LLDBSwigPythonCreateScriptedThreadPlan(
class_name, python_interpreter->m_dictionary_name.c_str(), args_data,
error_str, thread_plan_sp);
if (!ret_val)
return {};
return StructuredData::ObjectSP(new StructuredPythonObject(ret_val));
}
@ -1860,16 +1846,12 @@ ScriptInterpreterPythonImpl::CreateScriptedBreakpointResolver(
if (!python_interpreter)
return StructuredData::GenericSP();
void *ret_val;
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
{
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
ret_val = LLDBSwigPythonCreateScriptedBreakpointResolver(
class_name, python_interpreter->m_dictionary_name.c_str(), args_data,
bkpt_sp);
}
void *ret_val = LLDBSwigPythonCreateScriptedBreakpointResolver(
class_name, python_interpreter->m_dictionary_name.c_str(), args_data,
bkpt_sp);
return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
}
@ -1935,16 +1917,12 @@ StructuredData::GenericSP ScriptInterpreterPythonImpl::CreateScriptedStopHook(
return StructuredData::GenericSP();
}
void *ret_val;
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
{
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
ret_val = LLDBSwigPythonCreateScriptedStopHook(
target_sp, class_name, python_interpreter->m_dictionary_name.c_str(),
args_data, error);
}
void *ret_val = LLDBSwigPythonCreateScriptedStopHook(
target_sp, class_name, python_interpreter->m_dictionary_name.c_str(),
args_data, error);
return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
}
@ -2035,14 +2013,10 @@ ScriptInterpreterPythonImpl::CreateSyntheticScriptedProvider(
if (!python_interpreter)
return StructuredData::ObjectSP();
void *ret_val = nullptr;
{
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
ret_val = LLDBSwigPythonCreateSyntheticProvider(
class_name, python_interpreter->m_dictionary_name.c_str(), valobj);
}
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
void *ret_val = LLDBSwigPythonCreateSyntheticProvider(
class_name, python_interpreter->m_dictionary_name.c_str(), valobj);
return StructuredData::ObjectSP(new StructuredPythonObject(ret_val));
}
@ -2057,14 +2031,10 @@ ScriptInterpreterPythonImpl::CreateScriptCommandObject(const char *class_name) {
if (!debugger_sp.get())
return StructuredData::GenericSP();
void *ret_val;
{
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
ret_val = LLDBSwigPythonCreateCommandObject(
class_name, m_dictionary_name.c_str(), debugger_sp);
}
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
void *ret_val = LLDBSwigPythonCreateCommandObject(
class_name, m_dictionary_name.c_str(), debugger_sp);
return StructuredData::GenericSP(new StructuredPythonObject(ret_val));
}
@ -2176,8 +2146,11 @@ bool ScriptInterpreterPythonImpl::GetScriptedSummary(
return false;
}
if (new_callee && old_callee != new_callee)
if (new_callee && old_callee != new_callee) {
Locker py_lock(this,
Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
callee_wrapper_sp = std::make_shared<StructuredPythonObject>(new_callee);
}
return ret_val;
}