Switching back to using std::tr1::shared_ptr. We originally switched away
due to RTTI worries since llvm and clang don't use RTTI, but I was able to
switch back with no issues as far as I can tell. Once the RTTI issue wasn't
an issue, we were looking for a way to properly track weak pointers to objects
to solve some of the threading issues we have been running into which naturally
led us back to std::tr1::weak_ptr. We also wanted the ability to make a shared
pointer from just a pointer, which is also easily solved using the
std::tr1::enable_shared_from_this class.
The main reason for this move back is so we can start properly having weak
references to objects. Currently a lldb_private::Thread class has a refrence
to its parent lldb_private::Process. This doesn't work well when we now hand
out a SBThread object that contains a shared pointer to a lldb_private::Thread
as this SBThread can be held onto by external clients and if they end up
using one of these objects we can easily crash.
So the next task is to start adopting std::tr1::weak_ptr where ever it makes
sense which we can do with lldb_private::Debugger, lldb_private::Target,
lldb_private::Process, lldb_private::Thread, lldb_private::StackFrame, and
many more objects now that they are no longer using intrusive ref counted
pointer objects (you can't do std::tr1::weak_ptr functionality with intrusive
pointers).
git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@149207 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/source/Target/ExecutionContext.cpp b/source/Target/ExecutionContext.cpp
index 41cf04d..8360d1a 100644
--- a/source/Target/ExecutionContext.cpp
+++ b/source/Target/ExecutionContext.cpp
@@ -46,7 +46,7 @@
}
ExecutionContext::ExecutionContext (Target* t, bool fill_current_process_thread_frame) :
- m_target_sp (t),
+ m_target_sp (t->shared_from_this()),
m_process_sp (),
m_thread_sp (),
m_frame_sp ()
@@ -58,17 +58,19 @@
{
m_thread_sp = m_process_sp->GetThreadList().GetSelectedThread();
if (m_thread_sp)
- m_frame_sp = m_thread_sp->GetSelectedFrame().get();
+ m_frame_sp = m_thread_sp->GetSelectedFrame();
}
}
}
ExecutionContext::ExecutionContext(Process* process, Thread *thread, StackFrame *frame) :
- m_target_sp (process ? &process->GetTarget() : NULL),
- m_process_sp (process),
- m_thread_sp (thread),
- m_frame_sp (frame)
+ m_target_sp (),
+ m_process_sp (process->shared_from_this()),
+ m_thread_sp (thread->shared_from_this()),
+ m_frame_sp (frame->shared_from_this())
{
+ if (process)
+ m_target_sp = process->GetTarget().shared_from_this();
}
ExecutionContext::ExecutionContext (ExecutionContextScope *exe_scope_ptr)
@@ -200,24 +202,36 @@
void
ExecutionContext::SetTargetPtr (Target* target)
{
- m_target_sp = target;
+ if (target)
+ m_target_sp = target->shared_from_this();
+ else
+ m_target_sp.reset();
}
void
ExecutionContext::SetProcessPtr (Process *process)
{
- m_process_sp = process;
+ if (process)
+ m_process_sp = process->shared_from_this();
+ else
+ m_process_sp.reset();
}
void
ExecutionContext::SetThreadPtr (Thread *thread)
{
- m_thread_sp = thread;
+ if (thread)
+ m_thread_sp = thread->shared_from_this();
+ else
+ m_thread_sp.reset();
}
void
ExecutionContext::SetFramePtr (StackFrame *frame)
{
- m_frame_sp = frame;
+ if (frame)
+ m_frame_sp = frame->shared_from_this();
+ else
+ m_frame_sp.reset();
}
diff --git a/source/Target/Process.cpp b/source/Target/Process.cpp
index 2eb519f..e991437 100644
--- a/source/Target/Process.cpp
+++ b/source/Target/Process.cpp
@@ -848,7 +848,7 @@
m_abi_sp.reset();
m_os_ap.reset();
m_dyld_ap.reset();
- m_thread_list.Clear();
+ m_thread_list.Destroy();
std::vector<Notifications> empty_notifications;
m_notifications.swap(empty_notifications);
m_image_tokens.clear();
@@ -1543,7 +1543,7 @@
}
lldb::break_id_t
-Process::CreateBreakpointSite (BreakpointLocationSP &owner, bool use_hardware)
+Process::CreateBreakpointSite (const BreakpointLocationSP &owner, bool use_hardware)
{
const addr_t load_addr = owner->GetAddress().GetOpcodeLoadAddress (&m_target);
if (load_addr != LLDB_INVALID_ADDRESS)
@@ -3410,12 +3410,6 @@
exe_ctx.SetFramePtr (NULL);
}
-lldb::ProcessSP
-Process::GetSP ()
-{
- return GetTarget().GetProcessSP();
-}
-
//uint32_t
//Process::ListProcessesMatchingName (const char *name, StringList &matches, std::vector<lldb::pid_t> &pids)
//{
@@ -3680,13 +3674,10 @@
Process::UpdateInstanceName ()
{
Module *module = GetTarget().GetExecutableModulePointer();
- if (module)
+ if (module && module->GetFileSpec().GetFilename())
{
- StreamString sstr;
- sstr.Printf ("%s", module->GetFileSpec().GetFilename().AsCString());
-
GetSettingsController()->RenameInstanceSettings (GetInstanceName().AsCString(),
- sstr.GetData());
+ module->GetFileSpec().GetFilename().AsCString());
}
}
diff --git a/source/Target/StackFrame.cpp b/source/Target/StackFrame.cpp
index 666bc1d..2ce69f8 100644
--- a/source/Target/StackFrame.cpp
+++ b/source/Target/StackFrame.cpp
@@ -95,7 +95,7 @@
if (reg_context_sp && !m_sc.target_sp)
{
- m_sc.target_sp = reg_context_sp->GetThread().GetProcess().GetTarget().GetSP();
+ m_sc.target_sp = reg_context_sp->GetThread().GetProcess().GetTarget().shared_from_this();
m_flags.Set (eSymbolContextTarget);
}
}
@@ -129,16 +129,16 @@
if (m_sc.target_sp.get() == NULL && reg_context_sp)
{
- m_sc.target_sp = reg_context_sp->GetThread().GetProcess().GetTarget().GetSP();
+ m_sc.target_sp = reg_context_sp->GetThread().GetProcess().GetTarget().shared_from_this();
m_flags.Set (eSymbolContextTarget);
}
- Module *pc_module = pc_addr.GetModule();
+ Module *pc_module = pc_addr.GetModulePtr();
if (m_sc.module_sp.get() == NULL || m_sc.module_sp.get() != pc_module)
{
if (pc_module)
{
- m_sc.module_sp = pc_module;
+ m_sc.module_sp = pc_module->shared_from_this();
m_flags.Set (eSymbolContextModule);
}
else
@@ -218,7 +218,7 @@
Module *module = section->GetModule();
if (module)
{
- m_sc.module_sp = module;
+ m_sc.module_sp = module->shared_from_this();
if (m_sc.module_sp)
m_flags.Set(eSymbolContextModule);
}
@@ -417,7 +417,7 @@
// If the target was requested add that:
if (m_sc.target_sp.get() == NULL)
{
- m_sc.target_sp = CalculateProcess()->GetTarget().GetSP();
+ m_sc.target_sp = CalculateProcess()->GetTarget().shared_from_this();
if (m_sc.target_sp)
resolved |= eSymbolContextTarget;
}
@@ -1245,16 +1245,6 @@
return false;
}
-StackFrameSP
-StackFrame::GetSP ()
-{
- // This object contains an instrusive ref count base class so we can
- // easily make a shared pointer to this object
- return StackFrameSP (this);
-}
-
-
-
bool
StackFrame::GetStatus (Stream& strm,
bool show_frame_info,
diff --git a/source/Target/Target.cpp b/source/Target/Target.cpp
index 46ce1d3..f4159de 100644
--- a/source/Target/Target.cpp
+++ b/source/Target/Target.cpp
@@ -149,14 +149,6 @@
return m_process_sp;
}
-lldb::TargetSP
-Target::GetSP()
-{
- // This object contains an instrusive ref count base class so we can
- // easily make a shared pointer to this object
- return TargetSP(this);
-}
-
void
Target::Destroy()
{
@@ -256,8 +248,7 @@
BreakpointSP
Target::CreateBreakpoint (Address &addr, bool internal)
{
- TargetSP target_sp = this->GetSP();
- SearchFilterSP filter_sp(new SearchFilterForNonModuleSpecificSearches (target_sp));
+ SearchFilterSP filter_sp(new SearchFilterForNonModuleSpecificSearches (shared_from_this()));
BreakpointResolverSP resolver_sp (new BreakpointResolverAddress (NULL, addr));
return CreateBreakpoint (filter_sp, resolver_sp, internal);
}
@@ -290,17 +281,16 @@
Target::GetSearchFilterForModule (const FileSpec *containingModule)
{
SearchFilterSP filter_sp;
- lldb::TargetSP target_sp = this->GetSP();
if (containingModule != NULL)
{
// TODO: We should look into sharing module based search filters
// across many breakpoints like we do for the simple target based one
- filter_sp.reset (new SearchFilterByModule (target_sp, *containingModule));
+ filter_sp.reset (new SearchFilterByModule (shared_from_this(), *containingModule));
}
else
{
if (m_search_filter_sp.get() == NULL)
- m_search_filter_sp.reset (new SearchFilterForNonModuleSpecificSearches (target_sp));
+ m_search_filter_sp.reset (new SearchFilterForNonModuleSpecificSearches (shared_from_this()));
filter_sp = m_search_filter_sp;
}
return filter_sp;
@@ -310,17 +300,16 @@
Target::GetSearchFilterForModuleList (const FileSpecList *containingModules)
{
SearchFilterSP filter_sp;
- lldb::TargetSP target_sp = this->GetSP();
if (containingModules && containingModules->GetSize() != 0)
{
// TODO: We should look into sharing module based search filters
// across many breakpoints like we do for the simple target based one
- filter_sp.reset (new SearchFilterByModuleList (target_sp, *containingModules));
+ filter_sp.reset (new SearchFilterByModuleList (shared_from_this(), *containingModules));
}
else
{
if (m_search_filter_sp.get() == NULL)
- m_search_filter_sp.reset (new SearchFilterForNonModuleSpecificSearches (target_sp));
+ m_search_filter_sp.reset (new SearchFilterForNonModuleSpecificSearches (shared_from_this()));
filter_sp = m_search_filter_sp;
}
return filter_sp;
@@ -333,17 +322,16 @@
return GetSearchFilterForModuleList(containingModules);
SearchFilterSP filter_sp;
- lldb::TargetSP target_sp = this->GetSP();
if (containingModules == NULL)
{
// We could make a special "CU List only SearchFilter". Better yet was if these could be composable,
// but that will take a little reworking.
- filter_sp.reset (new SearchFilterByModuleListAndCU (target_sp, FileSpecList(), *containingSourceFiles));
+ filter_sp.reset (new SearchFilterByModuleListAndCU (shared_from_this(), FileSpecList(), *containingSourceFiles));
}
else
{
- filter_sp.reset (new SearchFilterByModuleListAndCU (target_sp, *containingModules, *containingSourceFiles));
+ filter_sp.reset (new SearchFilterByModuleListAndCU (shared_from_this(), *containingModules, *containingSourceFiles));
}
return filter_sp;
}
@@ -1073,11 +1061,12 @@
if (load_addr == LLDB_INVALID_ADDRESS)
{
- if (resolved_addr.GetModule() && resolved_addr.GetModule()->GetFileSpec())
+ Module *addr_module = resolved_addr.GetModulePtr();
+ if (addr_module && addr_module->GetFileSpec())
error.SetErrorStringWithFormat("%s[0x%llx] can't be resolved, %s in not currently loaded",
- resolved_addr.GetModule()->GetFileSpec().GetFilename().AsCString(),
+ addr_module->GetFileSpec().GetFilename().AsCString(),
resolved_addr.GetFileAddress(),
- resolved_addr.GetModule()->GetFileSpec().GetFilename().AsCString());
+ addr_module->GetFileSpec().GetFilename().AsCString());
else
error.SetErrorStringWithFormat("0x%llx can't be resolved", resolved_addr.GetFileAddress());
}
@@ -1339,7 +1328,7 @@
if (m_scratch_ast_context_ap.get() == NULL && m_arch.IsValid() && create_on_demand)
{
m_scratch_ast_context_ap.reset (new ClangASTContext(m_arch.GetTriple().str().c_str()));
- m_scratch_ast_source_ap.reset (new ClangASTSource(GetSP()));
+ m_scratch_ast_source_ap.reset (new ClangASTSource(shared_from_this()));
m_scratch_ast_source_ap->InstallASTContext(m_scratch_ast_context_ap->getASTContext());
llvm::OwningPtr<clang::ExternalASTSource> proxy_ast_source(m_scratch_ast_source_ap->CreateProxy());
m_scratch_ast_context_ap->SetExternalSource(proxy_ast_source);
@@ -1678,7 +1667,7 @@
Target::AddStopHook (Target::StopHookSP &new_hook_sp)
{
lldb::user_id_t new_uid = ++m_stop_hook_next_id;
- new_hook_sp.reset (new StopHook(GetSP(), new_uid));
+ new_hook_sp.reset (new StopHook(shared_from_this(), new_uid));
m_stop_hooks[new_uid] = new_hook_sp;
return new_uid;
}
diff --git a/source/Target/Thread.cpp b/source/Target/Thread.cpp
index ae083c7..5d7c905 100644
--- a/source/Target/Thread.cpp
+++ b/source/Target/Thread.cpp
@@ -1030,15 +1030,6 @@
&end);
}
-lldb::ThreadSP
-Thread::GetSP ()
-{
- // This object contains an instrusive ref count base class so we can
- // easily make a shared pointer to this object
- return ThreadSP(this);
-}
-
-
void
Thread::SettingsInitialize ()
{
diff --git a/source/Target/ThreadList.cpp b/source/Target/ThreadList.cpp
index f4d75f5..62eb1de 100644
--- a/source/Target/ThreadList.cpp
+++ b/source/Target/ThreadList.cpp
@@ -355,6 +355,17 @@
}
void
+ThreadList::Destroy()
+{
+ Mutex::Locker locker(m_threads_mutex);
+ const uint32_t num_threads = m_threads.size();
+ for (uint32_t idx = 0; idx < num_threads; ++idx)
+ {
+ m_threads[idx]->DestroyThread();
+ }
+}
+
+void
ThreadList::RefreshStateAfterStop ()
{
Mutex::Locker locker(m_threads_mutex);
@@ -603,6 +614,32 @@
m_stop_id = rhs.m_stop_id;
m_threads.swap(rhs.m_threads);
m_selected_tid = rhs.m_selected_tid;
+
+
+ // Now we look for threads that we are done with and
+ // make sure to clear them up as much as possible so
+ // anyone with a shared pointer will still have a reference,
+ // but the thread won't be of much use. Using std::weak_ptr
+ // for all backward references (such as a thread to a process)
+ // will eventually solve this issue for us, but for now, we
+ // need to work around the issue
+ collection::iterator rhs_pos, rhs_end = rhs.m_threads.end();
+ for (rhs_pos = rhs.m_threads.begin(); rhs_pos != rhs_end; ++rhs_pos)
+ {
+ const lldb::tid_t tid = (*rhs_pos)->GetID();
+ bool thread_is_alive = false;
+ const uint32_t num_threads = m_threads.size();
+ for (uint32_t idx = 0; idx < num_threads; ++idx)
+ {
+ if (m_threads[idx]->GetID() == tid)
+ {
+ thread_is_alive = true;
+ break;
+ }
+ }
+ if (!thread_is_alive)
+ (*rhs_pos)->DestroyThread();
+ }
}
}
diff --git a/source/Target/ThreadPlanTestCondition.cpp b/source/Target/ThreadPlanTestCondition.cpp
index 42aa01c..b9f8a27 100644
--- a/source/Target/ThreadPlanTestCondition.cpp
+++ b/source/Target/ThreadPlanTestCondition.cpp
@@ -119,7 +119,7 @@
else
{
// Now we have to change the event to a breakpoint event and mark it up appropriately:
- Process::ProcessEventData *new_data = new Process::ProcessEventData (m_thread.GetProcess().GetSP(), eStateStopped);
+ Process::ProcessEventData *new_data = new Process::ProcessEventData (m_thread.GetProcess().shared_from_this(), eStateStopped);
event_ptr->SetData(new_data);
event_ptr->SetType(Process::eBroadcastBitStateChanged);
SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID (m_thread,