Author: jdevlieghere Date: Fri Apr 13 03:25:23 2018 New Revision: 330002 URL: http://llvm.org/viewvc/llvm-project?rev=330002&view=rev Log: 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 Modified: lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp Modified: lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp?rev=330002&r1=330001&r2=330002&view=diff ============================================================================== --- lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp (original) +++ lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp Fri Apr 13 03:25:23 2018 @@ -156,9 +156,10 @@ bool OperatingSystemPython::UpdateThread 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. @@ -166,9 +167,10 @@ bool OperatingSystemPython::UpdateThread // 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 " @@ -176,12 +178,8 @@ bool OperatingSystemPython::UpdateThread 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::CreateRegisterCon 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::Cr 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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits