Prevent deadlock in OS Plugins

Summary:
When performing a synchronous resume, the API mutex is held until the
process is stopped. This is fine, except for when the OS plugins are processing
an event before the main thread is aware of it, in which case we end up with a
deadlock because in the internal thread we acquire a resource lock first, and
then wait for the API lock, while in the main thread we do the opposite, we
already hold the API mutex but are now waiting for the event mutex to handle
the event.

This patch fixes this by relaxing the need for the API lock in the OS plugins.
We can get away with this because we now this code is executed in the main
thread. As stated in the comment above, we just want to ensure nobody else
messes with the API while we're making a change. In theory it's possible that
the main thread would release the lock while we're executing the function, but
prevent this would require a more structural solution (which we want, but do
not have today).

The same workaround was already present, but this patch generalizes it to the
whole file.

This will allow me to re-land r329891.

Reviewers: clayborg, jingham, labath

Subscribers: llvm-commits, lldb-commits

Differential Revision: https://reviews.llvm.org/D45586

llvm-svn: 330002

lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp

@@ -156,32 +156,30 @@ bool OperatingSystemPython::UpdateThreadList(ThreadList &old_thread_list,
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_OS));
 
-  // First thing we have to do is to try to get the API lock, and the run lock.
-  // We're going to change the thread content of the process, and we're going
-  // to use python, which requires the API lock to do it.
+  // First thing we have to do is to try to get the API lock, and the
+  // interpreter lock. We're going to change the thread content of the process,
+  // and we're going to use python, which requires the API lock to do it. We
+  // need the interpreter lock to make sure thread_info_dict stays alive.
   //
   // If someone already has the API lock, that is ok, we just want to avoid
   // external code from making new API calls while this call is happening.
   //
   // This is a recursive lock so we can grant it to any Python code called on
   // the stack below us.
   Target &target = m_process->GetTarget();
-  std::unique_lock<std::recursive_mutex> lock(target.GetAPIMutex(),
-                                              std::defer_lock);
-  lock.try_lock();
+  std::unique_lock<std::recursive_mutex> api_lock(target.GetAPIMutex(),
+                                                  std::defer_lock);
+  api_lock.try_lock();
+  auto interpreter_lock = m_interpreter->AcquireInterpreterLock();
 
   if (log)
     log->Printf("OperatingSystemPython::UpdateThreadList() fetching thread "
                 "data from python for pid %" PRIu64,
                 m_process->GetID());
 
   // The threads that are in "new_thread_list" upon entry are the threads from
-  // the
-  // lldb_private::Process subclass, no memory threads will be in this list.
-
-  auto interpreter_lock =
-      m_interpreter
-          ->AcquireInterpreterLock(); // to make sure threads_list stays alive
+  // the lldb_private::Process subclass, no memory threads will be in this
+  // list.
   StructuredData::ArraySP threads_list =
       m_interpreter->OSPlugin_ThreadsInfo(m_python_object_sp);
 
@@ -301,20 +299,24 @@ OperatingSystemPython::CreateRegisterContextForThread(Thread *thread,
   if (!IsOperatingSystemPluginThread(thread->shared_from_this()))
     return reg_ctx_sp;
 
-  // First thing we have to do is get the API lock, and the run lock.  We're
-  // going to change the thread
-  // content of the process, and we're going to use python, which requires the
-  // API lock to do it.
-  // So get & hold that.  This is a recursive lock so we can grant it to any
-  // Python code called on the stack below us.
+  // First thing we have to do is to try to get the API lock, and the
+  // interpreter lock. We're going to change the thread content of the process,
+  // and we're going to use python, which requires the API lock to do it. We
+  // need the interpreter lock to make sure thread_info_dict stays alive.
+  //
+  // If someone already has the API lock, that is ok, we just want to avoid
+  // external code from making new API calls while this call is happening.
+  //
+  // This is a recursive lock so we can grant it to any Python code called on
+  // the stack below us.
   Target &target = m_process->GetTarget();
-  std::lock_guard<std::recursive_mutex> guard(target.GetAPIMutex());
+  std::unique_lock<std::recursive_mutex> api_lock(target.GetAPIMutex(),
+                                                  std::defer_lock);
+  api_lock.try_lock();
+  auto interpreter_lock = m_interpreter->AcquireInterpreterLock();
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
 
-  auto lock =
-      m_interpreter
-          ->AcquireInterpreterLock(); // to make sure python objects stays alive
   if (reg_data_addr != LLDB_INVALID_ADDRESS) {
     // The registers data is in contiguous memory, just create the register
     // context using the address provided
@@ -383,18 +385,23 @@ lldb::ThreadSP OperatingSystemPython::CreateThread(lldb::tid_t tid,
                 tid, context);
 
   if (m_interpreter && m_python_object_sp) {
-    // First thing we have to do is get the API lock, and the run lock.  We're
-    // going to change the thread
-    // content of the process, and we're going to use python, which requires the
-    // API lock to do it.
-    // So get & hold that.  This is a recursive lock so we can grant it to any
-    // Python code called on the stack below us.
+    // First thing we have to do is to try to get the API lock, and the
+    // interpreter lock. We're going to change the thread content of the
+    // process, and we're going to use python, which requires the API lock to
+    // do it. We need the interpreter lock to make sure thread_info_dict stays
+    // alive.
+    //
+    // If someone already has the API lock, that is ok, we just want to avoid
+    // external code from making new API calls while this call is happening.
+    //
+    // This is a recursive lock so we can grant it to any Python code called on
+    // the stack below us.
     Target &target = m_process->GetTarget();
-    std::lock_guard<std::recursive_mutex> guard(target.GetAPIMutex());
+    std::unique_lock<std::recursive_mutex> api_lock(target.GetAPIMutex(),
+                                                    std::defer_lock);
+    api_lock.try_lock();
+    auto interpreter_lock = m_interpreter->AcquireInterpreterLock();
 
-    auto lock = m_interpreter->AcquireInterpreterLock(); // to make sure
-                                                         // thread_info_dict
-                                                         // stays alive
     StructuredData::DictionarySP thread_info_dict =
         m_interpreter->OSPlugin_CreateThread(m_python_object_sp, tid, context);
     std::vector<bool> core_used_map;