[Lldb-commits] [PATCH] Prevent LLGS from crashing when exiting - make NativeProcessLinux to wait until ThreadStateCoordinator is fully stopped before entering ~NativeProcessLinux.

[Oleksiy Vyalov]

Added mutex lock in ~GDBRemoteCommunicationServerLLGS.

ThreadStateCoordinator references NativeProcessLinux indirectly:

- TSC thread function (NativeProcessLinux::CoordinatorThread, https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/Process/Linux/NativeProcessLinux.cpp#L3605) uses a process pointer
- NativeProcessLinux calls TSC methods passing lambda with this pointer - for example, https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/Process/Linux/NativeProcessLinux.cpp#L2116

I removed StopMonitor(); from ~NativeProcessLinux  and moved this call to NativeProcessLinux::Terminate but with next differences:

- Now StopMonitor() is called from NativeProcessLinux::Terminate before ~NativeProcessLinux - i.e., NativeProcessLinux is alive and can be safely referenced by TSC
- Without my change  StopMonitor() is called within ~NativeProcessLinux (so, NativeProcessLinux is the middle of destruction) after ~GDBRemoteCommunicationServerLLGS has been called.


http://reviews.llvm.org/D7692

Files:
  include/lldb/Host/common/NativeProcessProtocol.h
  source/Host/common/NativeProcessProtocol.cpp
  source/Plugins/Process/Linux/NativeProcessLinux.cpp
  source/Plugins/Process/Linux/NativeProcessLinux.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: include/lldb/Host/common/NativeProcessProtocol.h
===================================================================
--- include/lldb/Host/common/NativeProcessProtocol.h
+++ include/lldb/Host/common/NativeProcessProtocol.h
@@ -283,6 +283,10 @@
         bool
         UnregisterNativeDelegate (NativeDelegate &native_delegate);
 
+        // Called before termination of NativeProcessProtocol's instance.
+        virtual void
+        Terminate ();
+
     protected:
         lldb::pid_t m_pid;
 
Index: source/Host/common/NativeProcessProtocol.cpp
===================================================================
--- source/Host/common/NativeProcessProtocol.cpp
+++ source/Host/common/NativeProcessProtocol.cpp
@@ -435,3 +435,9 @@
 {
     // Default implementation does nothing.
 }
+
+void
+NativeProcessProtocol::Terminate ()
+{
+    // Default implementation does nothing.
+}
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1453,7 +1453,8 @@
     }
 }
 
-NativeProcessLinux::~NativeProcessLinux()
+void
+NativeProcessLinux::Terminate ()
 {
     StopMonitor();
 }
@@ -3637,6 +3638,7 @@
     // Tell the coordinator we're done.  This will cause the coordinator
     // run loop thread to exit when the processing queue hits this message.
     m_coordinator_up->StopCoordinator ();
+    m_coordinator_thread.Join (nullptr);
 }
 
 bool
Index: source/Plugins/Process/Linux/NativeProcessLinux.h
===================================================================
--- source/Plugins/Process/Linux/NativeProcessLinux.h
+++ source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -62,12 +62,6 @@
             NativeProcessProtocolSP &native_process_sp);
 
         // ---------------------------------------------------------------------
-        // Public Instance Methods
-        // ---------------------------------------------------------------------
-
-        ~NativeProcessLinux() override;
-
-        // ---------------------------------------------------------------------
         // NativeProcessProtocol Interface
         // ---------------------------------------------------------------------
         Error
@@ -118,6 +112,9 @@
         void
         DoStopIDBumped (uint32_t newBumpId) override;
 
+        void
+        Terminate () override;
+
         // ---------------------------------------------------------------------
         // Interface used by NativeRegisterContext-derived classes.
         // ---------------------------------------------------------------------
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -99,6 +99,13 @@
 //----------------------------------------------------------------------
 GDBRemoteCommunicationServerLLGS::~GDBRemoteCommunicationServerLLGS()
 {
+    Mutex::Locker locker (m_debugged_process_mutex);
+
+    if (m_debugged_process_sp)
+    {
+        m_debugged_process_sp->Terminate ();
+        m_debugged_process_sp.reset ();
+    }
 }
 
 void

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7692.20092.patch
Type: text/x-patch
Size: 3355 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150217/6f016da9/attachment.bin>