Fix a stall in running quit while a live process is running

lldb/include/lldb/Target/Process.h

@@ -558,7 +558,10 @@ class Process : public std::enable_shared_from_this<Process>,
   ///
   /// Subclasses that override this method should always call this superclass
   /// method.
-  virtual void Finalize();
+  /// If you are running Finalize in your Process subclass Destructor, pass
+  /// \b true.  If we are in the destructor, shared_from_this will no longer
+  /// work, so we have to avoid doing anything that might trigger that.
+  virtual void Finalize(bool destructing);
 
   /// Return whether this object is valid (i.e. has not been finalized.)
   ///
@@ -3079,6 +3082,11 @@ void PruneThreadPlans();
   /// This is set at the beginning of Process::Finalize() to stop functions
   /// from looking up or creating things during or after a finalize call.
   std::atomic<bool> m_finalizing;
+  // When we are "Finalizing" we need to do some cleanup.  But if the Finalize
+  // call is coming in the Destructor, we can't do any actual work in the
+  // process because that is likely to call "shared_from_this" which crashes
+  // if run while destructing.  We use this flag to determine that.
+  std::atomic<bool> m_destructing;
 
   /// Mask for code an data addresses. The default value (0) means no mask is
   /// set.  The bits set to 1 indicate bits that are NOT significant for

lldb/source/Core/Debugger.cpp

@@ -930,7 +930,7 @@ void Debugger::Clear() {
     for (TargetSP target_sp : m_target_list.Targets()) {
       if (target_sp) {
         if (ProcessSP process_sp = target_sp->GetProcessSP())
-          process_sp->Finalize();
+          process_sp->Finalize(false /* not destructing */);
         target_sp->Destroy();
       }
     }

lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp

@@ -164,7 +164,7 @@ ProcessKDP::~ProcessKDP() {
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
-  Finalize();
+  Finalize(true /* destructing */);
 }
 
 Status ProcessKDP::DoWillLaunch(Module *module) {

lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp

@@ -108,7 +108,7 @@ ProcessElfCore::~ProcessElfCore() {
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
-  Finalize();
+  Finalize(true /* destructing */);
 }
 
 lldb::addr_t ProcessElfCore::AddAddressRangeFromLoadSegment(

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -303,7 +303,7 @@ ProcessGDBRemote::~ProcessGDBRemote() {
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
-  Finalize();
+  Finalize(true /* destructing */);
 
   // The general Finalize is going to try to destroy the process and that
   // SHOULD shut down the async thread.  However, if we don't kill it it will

lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp

@@ -123,7 +123,7 @@ ProcessMachCore::~ProcessMachCore() {
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
-  Finalize();
+  Finalize(true /* destructing */);
 }
 
 bool ProcessMachCore::CheckAddressForDyldOrKernel(lldb::addr_t addr,

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

@@ -166,7 +166,7 @@ ProcessMinidump::~ProcessMinidump() {
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
-  Finalize();
+  Finalize(true /* destructing */);
 }
 
 void ProcessMinidump::Initialize() {

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

@@ -140,7 +140,7 @@ ScriptedProcess::~ScriptedProcess() {
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
-  Finalize();
+  Finalize(true /* destructing */);
 }
 
 void ScriptedProcess::Initialize() {

lldb/source/Target/Process.cpp

@@ -445,7 +445,7 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp,
       m_memory_cache(*this), m_allocated_memory_cache(*this),
       m_should_detach(false), m_next_event_action_up(), m_public_run_lock(),
       m_private_run_lock(), m_currently_handling_do_on_removals(false),
-      m_resume_requested(false), m_finalizing(false),
+      m_resume_requested(false), m_finalizing(false), m_destructing(false),
       m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false),
       m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false),
       m_can_interpret_function_calls(false), m_run_thread_plan_lock(),
@@ -518,9 +518,11 @@ ProcessProperties &Process::GetGlobalProperties() {
   return *g_settings_ptr;
 }
 
-void Process::Finalize() {
+void Process::Finalize(bool destructing) {
   if (m_finalizing.exchange(true))
     return;
+  if (destructing)
+    m_destructing.exchange(true);
 
   // Destroy the process. This will call the virtual function DoDestroy under
   // the hood, giving our derived class a chance to do the ncessary tear down.
@@ -1415,7 +1417,13 @@ bool Process::StateChangedIsHijackedForSynchronousResume() {
 StateType Process::GetPrivateState() { return m_private_state.GetValue(); }
 
 void Process::SetPrivateState(StateType new_state) {
-  if (m_finalizing)
+  // Use m_destructing not m_finalizing here.  If we are finalizing a process
+  // that we haven't started tearing down, we'd like to be able to nicely
+  // detach if asked, but that requires the event system be live.  That will
+  // not be true for an in-the-middle-of-being-destructed Process, since the
+  // event system relies on Process::shared_from_this, which may have already
+  // been destroyed.
+  if (m_destructing)
     return;
 
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process | LLDBLog::Unwind));

lldb/source/Target/ProcessTrace.cpp

@@ -50,7 +50,7 @@ ProcessTrace::~ProcessTrace() {
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
-  Finalize();
+  Finalize(true /* destructing */);
 }
 
 void ProcessTrace::DidAttach(ArchSpec &process_arch) {

lldb/source/Target/Target.cpp

@@ -197,7 +197,7 @@ void Target::DeleteCurrentProcess() {
     if (m_process_sp->IsAlive())
       m_process_sp->Destroy(false);
 
-    m_process_sp->Finalize();
+    m_process_sp->Finalize(false /* not destructing */);
 
     CleanupProcess();
 

lldb/test/API/driver/quit_speed/Makefile

@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/driver/quit_speed/TestQuitWithProcess.py

@@ -0,0 +1,34 @@
+"""
+Test that killing the target while quitting doesn't stall
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import pexpect
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+
+class DriverQuitSpeedTest(PExpectTest):
+    source = "main.c"
+
+    def test_run_quit(self):
+        """Test that the lldb driver's batch mode works correctly."""
+        self.build()
+
+        exe = self.getBuildArtifact("a.out")
+
+        # Turn on auto-confirm removes the wait for the prompt.
+        self.launch(executable=exe, extra_args=["-O", "settings set auto-confirm 1"])
+        child = self.child
+
+        # Launch the process without a TTY so we don't have to interrupt:
+        child.sendline("process launch -n")
+        print("launched process")
+        child.expect("Process ([\d]*) launched:")
+        print("Got launch message")
+        child.sendline("quit")
+        print("sent quit")
+        child.expect(pexpect.EOF, timeout=15)

lldb/test/API/driver/quit_speed/main.c

@@ -0,0 +1,8 @@
+#include <unistd.h>
+
+int main (int argc, char **argv) {
+  while(1)
+    usleep(5);
+
+  return 0;
+}