[lldb] Only use PyConfig when LLDB_EMBED_PYTHON_HOME is enabled

[JDevlieghere]

PyConfig and friends are not part of the stable API. We could switch back to Py_SetPythonHome, which has been deprecated, but still part of the stable API. For now, limit the use of PyConfig to when LLDB_EMBED_PYTHON_HOME is enabled, which essentially means Windows. Changing the order doesn't seem to matter.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

PyConfig and friends are not part of the stable API. We could switch back to Py_SetPythonHome, which has been deprecated, but still part of the stable API. For now, limit the use of PyConfig to when LLDB_EMBED_PYTHON_HOME is enabled, which essentially means Windows. Changing the order doesn't seem to matter.

---

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

1 Files Affected:

• (modified) lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (+21-19)

diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 24d604f22a765..5b97fcb5acf58 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -92,25 +92,6 @@ namespace {
 struct InitializePythonRAII {
 public:
   InitializePythonRAII() {
-    PyConfig config;
-    PyConfig_InitPythonConfig(&config);
-
-#if LLDB_EMBED_PYTHON_HOME
-    static std::string g_python_home = []() -> std::string {
-      if (llvm::sys::path::is_absolute(LLDB_PYTHON_HOME))
-        return LLDB_PYTHON_HOME;
-
-      FileSpec spec = HostInfo::GetShlibDir();
-      if (!spec)
-        return {};
-      spec.AppendPathComponent(LLDB_PYTHON_HOME);
-      return spec.GetPath();
-    }();
-    if (!g_python_home.empty()) {
-      PyConfig_SetBytesString(&config, &config.home, g_python_home.c_str());
-    }
-#endif
-
     // The table of built-in modules can only be extended before Python is
     // initialized.
     if (!Py_IsInitialized()) {
@@ -134,9 +115,30 @@ struct InitializePythonRAII {
       PyImport_AppendInittab("_lldb", LLDBSwigPyInit);
     }
 
+#if LLDB_EMBED_PYTHON_HOME
+    PyConfig config;
+    PyConfig_InitPythonConfig(&config);
+
+    static std::string g_python_home = []() -> std::string {
+      if (llvm::sys::path::is_absolute(LLDB_PYTHON_HOME))
+        return LLDB_PYTHON_HOME;
+
+      FileSpec spec = HostInfo::GetShlibDir();
+      if (!spec)
+        return {};
+      spec.AppendPathComponent(LLDB_PYTHON_HOME);
+      return spec.GetPath();
+    }();
+    if (!g_python_home.empty()) {
+      PyConfig_SetBytesString(&config, &config.home, g_python_home.c_str());
+    }
+
     config.install_signal_handlers = 0;
     Py_InitializeFromConfig(&config);
     PyConfig_Clear(&config);
+#else
+    Py_InitializeEx(/*install_sigs=*/0);
+#endif
 
     // The only case we should go further and acquire the GIL: it is unlocked.
     PyGILState_STATE gil_state = PyGILState_Ensure();

[medismailben]

Was moving the initialization past the if (!Py_IsInitialized()) { needed ?

[JDevlieghere]

Part of it, yes, but you could avoid changing the order by splitting the ifdef-ed code in two, which is slightly messier. I enabled LLDB_EMBED_PYTHON_HOME on Darwin and there it doesn't matter. That would look like this:

#if LLDB_EMBED_PYTHON_HOME
  PyConfig config;
  [...]
  PyConfig_SetBytesString(&config, &config.home, g_python_home.c_str());
#endif 

  if (!Py_IsInitialized()) {
  	[...]
    PyImport_AppendInittab("_lldb", LLDBSwigPyInit);
  }
  
#if LLDB_EMBED_PYTHON_HOME  
    config.install_signal_handlers = 0;
    Py_InitializeFromConfig(&config);
    PyConfig_Clear(&config);
#else
    Py_InitializeEx(/*install_sigs=*/0);
#endif

[JDevlieghere]

Part of #151617

[JDevlieghere]

Let's see how this does on Windows. If the bots complain I'll split the block to avoid the reordering.