[lldb] Make Python initialization atomic

[JDevlieghere]

Summary

We got a few crash reports that showed LLDB initializing Python on two separate threads. Make sure Python is initialized exactly once.

rdar://87287005

[mib]

LGTM

[labath]

I don't think that atomic<bool> is what you want here. In the case of a race, the "loser" will immediately continue to use python as if it was initialized, even though the winner has not finished the initialization. You most likely need call_once semantics, blocking all threads until the initialization completes.

That said, I think think it would be better to do this initialization in the Initialize static function. Out of general cleanliness, but with a particular with a view towards the SIGINT patch. That way the initialization functions happens in a predictable and single-threaded context (as you can see, threads are hard), hopefully at a point where nobody will care that we're mucking with the SIGINT handlers.

[JDevlieghere]

In D117601#3253663, @labath wrote:

I don't think that atomic<bool> is what you want here. In the case of a race, the "loser" will immediately continue to use python as if it was initialized, even though the winner has not finished the initialization. You most likely need call_once semantics, blocking all threads until the initialization completes.

That said, I think think it would be better to do this initialization in the Initialize static function. Out of general cleanliness, but with a particular with a view towards the SIGINT patch. That way the initialization functions happens in a predictable and single-threaded context (as you can see, threads are hard), hopefully at a point where nobody will care that we're mucking with the SIGINT handlers.

Makes sense. I’ll update the patch tomorrow.

[JDevlieghere]

Do private initialization exactly once in ScriptInterpreterPython::Initialize

[JDevlieghere]

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

3172–3194

I just moved this into the existing anonymous namespace above.

[labath]

LG, thanks

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

 #endif
 
 static ScriptInterpreterPythonImpl *GetPythonInterpreter(Debugger &debugger) {
   ScriptInterpreter *script_interpreter =
       debugger.GetScriptInterpreter(true, lldb::eScriptLanguagePython);
   return static_cast<ScriptInterpreterPythonImpl *>(script_interpreter);
 }
 
-static bool g_initialized = false;
+static std::atomic<bool> g_initialized(false);
 
 namespace {
 
 // Initializing Python is not a straightforward process.  We cannot control
 // what external code may have done before getting to this point in LLDB,
 // including potentially having already initialized Python, so we need to do a
 // lot of work to ensure that the existing state of the system is maintained
 // across our initialization.  We do this by using an RAII pattern where we

 std::unique_ptr<ScriptInterpreterLocker>
 ScriptInterpreterPythonImpl::AcquireInterpreterLock() {
   std::unique_ptr<ScriptInterpreterLocker> py_lock(new Locker(
       this, Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN,
       Locker::FreeLock | Locker::TearDownSession));
   return py_lock;
 }
 
 #if LLDB_USE_PYTHON_SET_INTERRUPT
 namespace {
 /// Saves the current signal handler for the specified signal and restores
 /// it at the end of the current scope.
 struct RestoreSignalHandlerScope {
   /// The signal handler.
   struct sigaction m_prev_handler;
   int m_signal_code;
   RestoreSignalHandlerScope(int signal_code) : m_signal_code(signal_code) {
     // Initialize sigaction to their default state.
     std::memset(&m_prev_handler, 0, sizeof(m_prev_handler));
     // Don't install a new handler, just read back the old one.
     struct sigaction *new_handler = nullptr;
     int signal_err = ::sigaction(m_signal_code, new_handler, &m_prev_handler);
     lldbassert(signal_err == 0 && "sigaction failed to read handler");
   }
   ~RestoreSignalHandlerScope() {
     int signal_err = ::sigaction(m_signal_code, &m_prev_handler, nullptr);
     lldbassert(signal_err == 0 && "sigaction failed to restore old handler");
   }
 };
 } // namespace
 #endif
 
 void ScriptInterpreterPythonImpl::InitializePrivate() {
-  if (g_initialized)
+  bool initialized = false;
+  const bool exchanged =
+      g_initialized.compare_exchange_strong(initialized, true);
+  if (!exchanged)
     return;
 
-  g_initialized = true;
-
   LLDB_SCOPED_TIMER();
 
   // RAII-based initialization which correctly handles multiple-initialization,
   // version- specific differences among Python 2 and Python 3, and saving and
   // restoring various other pieces of state that can get mucked with during
   // initialization.
   InitializePythonRAII initialize_guard;