From 4e0fe8ab95763b90e5fed11bc9598d82a1316d01 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Thu, 12 Jul 2012 20:32:19 +0000 Subject: [PATCH] Fixed a case where the python interpreter could end up holding onto a previous lldb::SBProcess (probably in lldb.process) when run under Xcode. Prior to this fix, the lldb::SBProcess held onto a shared pointer to a lldb_private::Process. This in turn could cause the process to still have a thread list with stack frames. The stack frames would have module shared pointers in the lldb_private::SymbolContext objects. We also had issues with things staying in the shared module list too long when we found things by UUID (we didn't remove the out of date ModuleSP from the global module cache). Now all of this is fixed and everything goes away between runs. llvm-svn: 160140 --- lldb/include/lldb/API/SBProcess.h | 2 +- lldb/include/lldb/Core/ModuleList.h | 13 +++++++ .../lldb/Interpreter/CommandInterpreter.h | 6 +-- lldb/source/API/SBProcess.cpp | 18 ++++----- lldb/source/Core/ModuleList.cpp | 32 +++++++++++++-- .../source/Interpreter/CommandInterpreter.cpp | 39 ++++--------------- 6 files changed, 63 insertions(+), 47 deletions(-) diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index f8ad2946a00b..23eac8f990b0 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -216,7 +216,7 @@ protected: void SetSP (const lldb::ProcessSP &process_sp); - lldb::ProcessSP m_opaque_sp; + lldb::ProcessWP m_opaque_wp; }; } // namespace lldb diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h index 3c1c30f165eb..38d397b032b6 100644 --- a/lldb/include/lldb/Core/ModuleList.h +++ b/lldb/include/lldb/Core/ModuleList.h @@ -75,6 +75,19 @@ public: void Append (const lldb::ModuleSP &module_sp); + //------------------------------------------------------------------ + /// Append a module to the module list and remove any equivalent + /// modules. Equivalent modules are ones whose file, platform file + /// and architecture matches. + /// + /// Replaces the module to the collection. + /// + /// @param[in] module_sp + /// A shared pointer to a module to replace in this collection. + //------------------------------------------------------------------ + void + ReplaceEquivalent (const lldb::ModuleSP &module_sp); + bool AppendIfNeeded (const lldb::ModuleSP &module_sp); diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index dee71479b846..5706503aa872 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -296,10 +296,10 @@ public: return m_debugger; } - ExecutionContext & + ExecutionContext GetExecutionContext() { - return m_exe_ctx; + return m_exe_ctx_ref.Lock(); } void @@ -464,7 +464,7 @@ private: PreprocessCommand (std::string &command); Debugger &m_debugger; // The debugger session that this interpreter is associated with - ExecutionContext m_exe_ctx; // The current execution context to use when handling commands + ExecutionContextRef m_exe_ctx_ref; // The current execution context to use when handling commands bool m_synchronous_execution; bool m_skip_lldbinit_files; bool m_skip_app_init_files; diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 67027a6d9a55..b764e559f934 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -39,7 +39,7 @@ using namespace lldb_private; SBProcess::SBProcess () : - m_opaque_sp() + m_opaque_wp() { } @@ -49,13 +49,13 @@ SBProcess::SBProcess () : //---------------------------------------------------------------------- SBProcess::SBProcess (const SBProcess& rhs) : - m_opaque_sp (rhs.m_opaque_sp) + m_opaque_wp (rhs.m_opaque_wp) { } SBProcess::SBProcess (const lldb::ProcessSP &process_sp) : - m_opaque_sp (process_sp) + m_opaque_wp (process_sp) { } @@ -63,7 +63,7 @@ const SBProcess& SBProcess::operator = (const SBProcess& rhs) { if (this != &rhs) - m_opaque_sp = rhs.m_opaque_sp; + m_opaque_wp = rhs.m_opaque_wp; return *this; } @@ -83,26 +83,26 @@ SBProcess::GetBroadcasterClassName () lldb::ProcessSP SBProcess::GetSP() const { - return m_opaque_sp; + return m_opaque_wp.lock(); } void SBProcess::SetSP (const ProcessSP &process_sp) { - m_opaque_sp = process_sp; + m_opaque_wp = process_sp; } void SBProcess::Clear () { - m_opaque_sp.reset(); + m_opaque_wp.reset(); } bool SBProcess::IsValid() const { - return m_opaque_sp.get() != NULL; + return m_opaque_wp.lock().get() != NULL; } bool @@ -119,7 +119,7 @@ SBProcess::RemoteLaunch (char const **argv, LogSP log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_API)); if (log) { log->Printf ("SBProcess(%p)::RemoteLaunch (argv=%p, envp=%p, stdin=%s, stdout=%s, stderr=%s, working-dir=%s, launch_flags=0x%x, stop_at_entry=%i, &error (%p))...", - m_opaque_sp.get(), + m_opaque_wp.lock().get(), argv, envp, stdin_path ? stdin_path : "NULL", diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp index 44f34e62d825..d6f29a6b49e8 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/lldb/source/Core/ModuleList.cpp @@ -77,6 +77,32 @@ ModuleList::Append (const ModuleSP &module_sp) } } +void +ModuleList::ReplaceEquivalent (const ModuleSP &module_sp) +{ + if (module_sp) + { + Mutex::Locker locker(m_modules_mutex); + + // First remove any equivalent modules. Equivalent modules are modules + // whose path, platform path and architecture match. + ModuleSpec equivalent_module_spec (module_sp->GetFileSpec(), module_sp->GetArchitecture()); + equivalent_module_spec.GetPlatformFileSpec() = module_sp->GetPlatformFileSpec(); + + size_t idx = 0; + while (idx < m_modules.size()) + { + ModuleSP module_sp (m_modules[idx]); + if (module_sp->MatchesModuleSpec (equivalent_module_spec)) + m_modules.erase(m_modules.begin() + idx); + else + ++idx; + } + // Now add the new module to the list + m_modules.push_back(module_sp); + } +} + bool ModuleList::AppendIfNeeded (const ModuleSP &module_sp) { @@ -323,7 +349,7 @@ ModuleList::FindSymbolsWithNameAndType (const ConstString &name, return sc_list.GetSize() - initial_size; } - size_t +size_t ModuleList::FindSymbolsMatchingRegExAndType (const RegularExpression ®ex, lldb::SymbolType symbol_type, SymbolContextList &sc_list, @@ -727,7 +753,7 @@ ModuleList::GetSharedModule if (did_create_ptr) *did_create_ptr = true; - shared_module_list.Append(module_sp); + shared_module_list.ReplaceEquivalent(module_sp); return error; } } @@ -819,7 +845,7 @@ ModuleList::GetSharedModule if (did_create_ptr) *did_create_ptr = true; - shared_module_list.Append(module_sp); + shared_module_list.ReplaceEquivalent(module_sp); } else { diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 8d29133acc8c..2879ec45f8f2 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -1143,7 +1143,8 @@ CommandInterpreter::PreprocessCommand (std::string &command) { std::string expr_str (command, expr_content_start, end_backtick - expr_content_start); - Target *target = m_exe_ctx.GetTargetPtr(); + ExecutionContext exe_ctx(GetExecutionContext()); + Target *target = exe_ctx.GetTargetPtr(); // Get a dummy target to allow for calculator mode while processing backticks. // This also helps break the infinite loop caused when target is null. if (!target) @@ -1155,7 +1156,7 @@ CommandInterpreter::PreprocessCommand (std::string &command) const bool keep_in_memory = false; ValueObjectSP expr_result_valobj_sp; ExecutionResults expr_result = target->EvaluateExpression (expr_str.c_str(), - m_exe_ctx.GetFramePtr(), + exe_ctx.GetFramePtr(), eExecutionPolicyOnlyWhenNeeded, coerce_to_id, unwind_on_error, @@ -2228,7 +2229,8 @@ CommandInterpreter::GetPlatform (bool prefer_target_platform) PlatformSP platform_sp; if (prefer_target_platform) { - Target *target = m_exe_ctx.GetTargetPtr(); + ExecutionContext exe_ctx(GetExecutionContext()); + Target *target = exe_ctx.GetTargetPtr(); if (target) platform_sp = target->GetPlatform(); } @@ -2618,39 +2620,14 @@ CommandInterpreter::FindCommandsForApropos (const char *search_word, StringList void CommandInterpreter::UpdateExecutionContext (ExecutionContext *override_context) { - m_exe_ctx.Clear(); - if (override_context != NULL) { - m_exe_ctx = *override_context; + m_exe_ctx_ref = *override_context; } else { - TargetSP target_sp (m_debugger.GetSelectedTarget()); - if (target_sp) - { - m_exe_ctx.SetTargetSP (target_sp); - ProcessSP process_sp (target_sp->GetProcessSP()); - m_exe_ctx.SetProcessSP (process_sp); - if (process_sp && process_sp->IsAlive() && !process_sp->IsRunning()) - { - ThreadSP thread_sp (process_sp->GetThreadList().GetSelectedThread()); - if (thread_sp) - { - m_exe_ctx.SetThreadSP (thread_sp); - StackFrameSP frame_sp (thread_sp->GetSelectedFrame()); - if (!frame_sp) - { - frame_sp = thread_sp->GetStackFrameAtIndex (0); - // If we didn't have a selected frame select one here. - if (frame_sp) - thread_sp->SetSelectedFrame(frame_sp.get()); - } - if (frame_sp) - m_exe_ctx.SetFrameSP (frame_sp); - } - } - } + const bool adopt_selected = true; + m_exe_ctx_ref.SetTargetPtr (m_debugger.GetSelectedTarget().get(), adopt_selected); } }