Author: sas Date: Thu Mar 17 13:52:41 2016 New Revision: 263735 URL: http://llvm.org/viewvc/llvm-project?rev=263735&view=rev Log: Fix deadlock due to thread list locking in 'bt all' with obj-c
Summary: The gdb-remote async thread cannot modify thread state while the main thread holds a lock on the state. Don't use locking thread iteration for bt all. Specifically, the deadlock manifests when lldb attempts to JIT code to symbolicate objective c while backtracing. As part of this code path, SetPrivateState() is called on an async thread. This async thread will block waiting for the thread_list lock held by the main thread in CommandObjectIterateOverThreads. The main thread will also block on the async thread during DoResume (although with a timeout), leading to a deadlock. Due to the timeout, the deadlock is not immediately apparent, but the inferior will be left in an invalid state after the bt all completes, and objective-c symbols will not be successfully resolved in the backtrace. Reviewers: andrew.w.kaylor, jingham, clayborg Subscribers: sas, lldb-commits Differential Revision: http://reviews.llvm.org/D18075 Change by Francis Ricci <fjri...@fb.com> Modified: lldb/trunk/source/Commands/CommandObjectThread.cpp Modified: lldb/trunk/source/Commands/CommandObjectThread.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectThread.cpp?rev=263735&r1=263734&r2=263735&view=diff ============================================================================== --- lldb/trunk/source/Commands/CommandObjectThread.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectThread.cpp Thu Mar 17 13:52:41 2016 @@ -66,34 +66,33 @@ public: if (command.GetArgumentCount() == 0) { Thread *thread = m_exe_ctx.GetThreadPtr(); - if (!HandleOneThread (*thread, result)) + if (!HandleOneThread (thread->GetID(), result)) return false; + return result.Succeeded(); } - else if (command.GetArgumentCount() == 1 && ::strcmp (command.GetArgumentAtIndex(0), "all") == 0) + + // Use tids instead of ThreadSPs to prevent deadlocking problems which result from JIT-ing + // code while iterating over the (locked) ThreadSP list. + std::vector<lldb::tid_t> tids; + + if (command.GetArgumentCount() == 1 && ::strcmp (command.GetArgumentAtIndex(0), "all") == 0) { Process *process = m_exe_ctx.GetProcessPtr(); - uint32_t idx = 0; - for (ThreadSP thread_sp : process->Threads()) - { - if (idx != 0 && m_add_return) - result.AppendMessage(""); - if (!HandleOneThread(*(thread_sp.get()), result)) - return false; - ++idx; - } + for (ThreadSP thread_sp : process->Threads()) + tids.push_back(thread_sp->GetID()); } else { const size_t num_args = command.GetArgumentCount(); Process *process = m_exe_ctx.GetProcessPtr(); + Mutex::Locker locker (process->GetThreadList().GetMutex()); - std::vector<ThreadSP> thread_sps; for (size_t i = 0; i < num_args; i++) { bool success; - + uint32_t thread_idx = StringConvert::ToUInt32(command.GetArgumentAtIndex(i), 0, 0, &success); if (!success) { @@ -101,26 +100,31 @@ public: result.SetStatus (eReturnStatusFailed); return false; } - - thread_sps.push_back(process->GetThreadList().FindThreadByIndexID(thread_idx)); - - if (!thread_sps[i]) + + ThreadSP thread = process->GetThreadList().FindThreadByIndexID(thread_idx); + + if (!thread) { result.AppendErrorWithFormat ("no thread with index: \"%s\"\n", command.GetArgumentAtIndex(i)); result.SetStatus (eReturnStatusFailed); return false; } - } - - for (uint32_t i = 0; i < num_args; i++) - { - if (!HandleOneThread (*(thread_sps[i].get()), result)) - return false; - if (i < num_args - 1 && m_add_return) - result.AppendMessage(""); + tids.push_back(thread->GetID()); } } + + uint32_t idx = 0; + for (const lldb::tid_t &tid : tids) + { + if (idx != 0 && m_add_return) + result.AppendMessage(""); + + if (!HandleOneThread (tid, result)) + return false; + + ++idx; + } return result.Succeeded(); } @@ -133,7 +137,7 @@ protected: // If m_add_return is true, a blank line will be inserted between each of the listings (except the last one.) virtual bool - HandleOneThread (Thread &thread, CommandReturnObject &result) = 0; + HandleOneThread (lldb::tid_t, CommandReturnObject &result) = 0; ReturnStatus m_success_return = eReturnStatusSuccessFinishResult; bool m_add_return = true; @@ -275,25 +279,35 @@ protected: } bool - HandleOneThread (Thread &thread, CommandReturnObject &result) override + HandleOneThread (lldb::tid_t tid, CommandReturnObject &result) override { + ThreadSP thread_sp = m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid); + if (!thread_sp) + { + result.AppendErrorWithFormat ("thread disappeared while computing backtraces: 0x%" PRIx64 "\n", tid); + result.SetStatus (eReturnStatusFailed); + return false; + } + + Thread *thread = thread_sp.get(); + Stream &strm = result.GetOutputStream(); // Don't show source context when doing backtraces. const uint32_t num_frames_with_source = 0; - if (!thread.GetStatus (strm, - m_options.m_start, - m_options.m_count, - num_frames_with_source)) + if (!thread->GetStatus (strm, + m_options.m_start, + m_options.m_count, + num_frames_with_source)) { - result.AppendErrorWithFormat ("error displaying backtrace for thread: \"0x%4.4x\"\n", thread.GetIndexID()); + result.AppendErrorWithFormat ("error displaying backtrace for thread: \"0x%4.4x\"\n", thread->GetIndexID()); result.SetStatus (eReturnStatusFailed); return false; } if (m_options.m_extended_backtrace) { - DoExtendedBacktrace (&thread, result); + DoExtendedBacktrace (thread, result); } return true; @@ -1537,12 +1551,22 @@ public: } bool - HandleOneThread (Thread &thread, CommandReturnObject &result) override + HandleOneThread (lldb::tid_t tid, CommandReturnObject &result) override { + ThreadSP thread_sp = m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid); + if (!thread_sp) + { + result.AppendErrorWithFormat ("thread no longer exists: 0x%" PRIx64 "\n", tid); + result.SetStatus (eReturnStatusFailed); + return false; + } + + Thread *thread = thread_sp.get(); + Stream &strm = result.GetOutputStream(); - if (!thread.GetDescription (strm, eDescriptionLevelFull, m_options.m_json_thread, m_options.m_json_stopinfo)) + if (!thread->GetDescription (strm, eDescriptionLevelFull, m_options.m_json_thread, m_options.m_json_stopinfo)) { - result.AppendErrorWithFormat ("error displaying info for thread: \"%d\"\n", thread.GetIndexID()); + result.AppendErrorWithFormat ("error displaying info for thread: \"%d\"\n", thread->GetIndexID()); result.SetStatus (eReturnStatusFailed); return false; } @@ -2044,14 +2068,24 @@ public: protected: bool - HandleOneThread (Thread &thread, CommandReturnObject &result) override + HandleOneThread (lldb::tid_t tid, CommandReturnObject &result) override { + ThreadSP thread_sp = m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid); + if (!thread_sp) + { + result.AppendErrorWithFormat ("thread no longer exists: 0x%" PRIx64 "\n", tid); + result.SetStatus (eReturnStatusFailed); + return false; + } + + Thread *thread = thread_sp.get(); + Stream &strm = result.GetOutputStream(); DescriptionLevel desc_level = eDescriptionLevelFull; if (m_options.m_verbose) desc_level = eDescriptionLevelVerbose; - thread.DumpThreadPlans (&strm, desc_level, m_options.m_internal, true); + thread->DumpThreadPlans (&strm, desc_level, m_options.m_internal, true); return true; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
