[lldb] Eliminate InitializePythonRAII::InitializeThreadsPrivate (NFC)

[JDevlieghere]

The behavior of thread initialization changed in Python 3.7. The minimum supported Python version is now 3.8. That means that PyEval_ThreadsInitialized always returns true and PyEval_InitThreads is never called.

The helper function existed to coordinate initializing the threads and acquiring the GIL, which is no longer necessary. With that, there's no point in having the helper at all. This PR eliminates the function and inlines to GIL acquisition into the caller.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The behavior of thread initialization changed in Python 3.7. The minimum supported Python version is now 3.8. That means that PyEval_ThreadsInitialized always returns true and PyEval_InitThreads is never called.

The helper function existed to coordinate initializing the threads and acquiring the GIL, which is no longer necessary. With that, there's no point in having the helper at all. This PR eliminates the function and inlines to GIL acquisition into the caller.

---

Full diff: https://github.com/llvm/llvm-project/pull/151780.diff

1 Files Affected:

• (modified) lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (+10-41)

diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index ce775690b02bf..34193b5d3ac8b 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -137,7 +137,16 @@ struct InitializePythonRAII {
     config.install_signal_handlers = 0;
     Py_InitializeFromConfig(&config);
     PyConfig_Clear(&config);
-    InitializeThreadsPrivate();
+
+    // The only case we should go further and acquire the GIL: it is unlocked.
+    if (PyGILState_Check())
+      return;
+
+    m_was_already_initialized = true;
+    m_gil_state = PyGILState_Ensure();
+    LLDB_LOGV(GetLog(LLDBLog::Script),
+              "Ensured PyGILState. Previous state = {0}locked\n",
+              m_gil_state == PyGILState_UNLOCKED ? "un" : "");
   }
 
   ~InitializePythonRAII() {
@@ -153,46 +162,6 @@ struct InitializePythonRAII {
   }
 
 private:
-  void InitializeThreadsPrivate() {
-    // Since Python 3.7 `Py_Initialize` calls `PyEval_InitThreads` inside
-    // itself, so there is no way to determine whether the embedded interpreter
-    // was already initialized by some external code.
-    // `PyEval_ThreadsInitialized` would always return `true` and
-    // `PyGILState_Ensure/Release` flow would be executed instead of unlocking
-    // GIL with `PyEval_SaveThread`. When an another thread calls
-    // `PyGILState_Ensure` it would get stuck in deadlock.
-
-    // The only case we should go further and acquire the GIL: it is unlocked.
-    if (PyGILState_Check())
-      return;
-
-// `PyEval_ThreadsInitialized` was deprecated in Python 3.9 and removed in
-// Python 3.13. It has been returning `true` always since Python 3.7.
-#if PY_VERSION_HEX < 0x03090000
-    if (PyEval_ThreadsInitialized()) {
-#else
-    if (true) {
-#endif
-      Log *log = GetLog(LLDBLog::Script);
-
-      m_was_already_initialized = true;
-      m_gil_state = PyGILState_Ensure();
-      LLDB_LOGV(log, "Ensured PyGILState. Previous state = {0}locked\n",
-                m_gil_state == PyGILState_UNLOCKED ? "un" : "");
-
-// `PyEval_InitThreads` was deprecated in Python 3.9 and removed in
-// Python 3.13.
-#if PY_VERSION_HEX < 0x03090000
-      return;
-    }
-
-    // InitThreads acquires the GIL if it hasn't been called before.
-    PyEval_InitThreads();
-#else
-    }
-#endif
-  }
-
   PyGILState_STATE m_gil_state = PyGILState_UNLOCKED;
   bool m_was_already_initialized = false;
 };

[JDevlieghere]

This is in preparation for eliminating the call to PyGILState_Check and calling PyGILState_Ensure directly as part of #151617.

[medismailben]

nit: this is more readable.

Suggested change

	"Ensured PyGILState. Previous state = {0}locked\n",
	m_gil_state == PyGILState_UNLOCKED ? "un" : "");
	"Ensured PyGILState. Previous state = {0}\n",
	m_gil_state == PyGILState_UNLOCKED ? "unlocked" : "locked");

[JDevlieghere]

Agreed that this is better, but there's 3 other instances, 2 of which are unrelated so I'll do it separately. 👍

[medismailben]

LGTM with nit.