jingham created this revision.
jingham added a reviewer: clayborg.
Herald added subscribers: lldb-commits, abidh.
Because of the way we use weak pointers in Process, it is not safe to just
destroy a fully live Process object. For instance, because the Thread holds a
ProcessWP, if the Process gets destroyed as a result of its SharedPointer count
going to 0, the thread can't get back to it's Process anymore (the WP->SP
conversion fails), and since it gets to the Target from the Process, it can't
get its target either.
This is structurally weak, but we've worked around it so far by making sure we
call Finalize on the Process before we allow it to be destroyed. Finalize
clears out all of the objects Process owns, and then the eventual destruction
can be just reclamation of memory.
However, we shot ourselves in the foot in Target::Launch by storing away a
ProcessWP for the previous process owned by the target, then setting our
m_process_sp to the new process THEN reconstituting the ProcessWP and calling
Finalize on it. But if Target was the last holder of the old ProcessSP, then
when we set m_process_sp to the new process, that would trigger the destruction
of the old Process BEFORE finalizing it. Tearing down the Process before
finalizing it doesn't always crash, it depends on what work the process was
doing at the time. But sometimes it does crash.
This patch avoids this problem by first finalizing the old process, THEN
resetting the shared pointer. That way we know Finalize will happen before
destruction.
This is a NFC commit, it only fixes a fairly obscure crash.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D55631
Files:
source/Target/Target.cpp
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2868,8 +2868,6 @@
ProcessWP process_wp;
if (m_process_sp)
process_wp = m_process_sp;
- m_process_sp =
- GetPlatform()->DebugProcess(launch_info, debugger, this, error);
// Cleanup the old process since someone might still have a strong
// reference to this process and we would like to allow it to cleanup as
@@ -2880,6 +2878,10 @@
ProcessSP old_process_sp(process_wp.lock());
if (old_process_sp)
old_process_sp->Finalize();
+
+ m_process_sp =
+ GetPlatform()->DebugProcess(launch_info, debugger, this, error);
+
} else {
if (log)
log->Printf("Target::%s the platform doesn't know how to debug a "
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2868,8 +2868,6 @@
ProcessWP process_wp;
if (m_process_sp)
process_wp = m_process_sp;
- m_process_sp =
- GetPlatform()->DebugProcess(launch_info, debugger, this, error);
// Cleanup the old process since someone might still have a strong
// reference to this process and we would like to allow it to cleanup as
@@ -2880,6 +2878,10 @@
ProcessSP old_process_sp(process_wp.lock());
if (old_process_sp)
old_process_sp->Finalize();
+
+ m_process_sp =
+ GetPlatform()->DebugProcess(launch_info, debugger, this, error);
+
} else {
if (log)
log->Printf("Target::%s the platform doesn't know how to debug a "
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
