[Lldb-commits] [lldb] r238089 - Did some cleanup to stop us from leaking Pipe file descriptors.

[Greg Clayton]

Author: gclayton
Date: Fri May 22 22:54:53 2015
New Revision: 238089

URL: http://llvm.org/viewvc/llvm-project?rev=238089&view=rev
Log:
Did some cleanup to stop us from leaking Pipe file descriptors.

The main issue was the Communication::Disconnect() was calling its Connection::Disconnect() but this wouldn't release the pipes that the ConnectionFileDescriptor was using. We also have someone that is holding a strong reference to the Process so that when you re-run, target replaces its m_process_sp, but it doesn't get destructed because someone has a strong reference to it. I need to track that down. But, even if we have a strong reference to the a process that is outstanding, we need to call Process::Finalize() to have it release as much of its resources as possible to avoid memory bloat. 

Removed the ProcessGDBRemote::SetExitStatus() override and replaced it with ProcessGDBRemote::DidExit().

Now we aren't leaking file descriptors and the stand alone test suite should run much better.



Modified:
    lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
    lldb/trunk/source/Target/Process.cpp
    lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp?rev=238089&r1=238088&r2=238089&view=diff
==============================================================================
--- lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp (original)
+++ lldb/trunk/source/Host/posix/ConnectionFileDescriptorPosix.cpp Fri May 22 22:54:53 2015
@@ -372,6 +372,9 @@ ConnectionFileDescriptor::Disconnect(Err
     if (error_ptr)
         *error_ptr = error.Fail() ? error : error2;
 
+    // Close any pipes we were using for async interrupts
+    m_pipe.Close();
+
     m_uri.clear();
     m_shutting_down = false;
     return status;

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=238089&r1=238088&r2=238089&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Fri May 22 22:54:53 2015
@@ -1401,12 +1401,11 @@ ProcessGDBRemote::DoAttachToProcessWithN
     return error;
 }
 
-
-bool
-ProcessGDBRemote::SetExitStatus (int exit_status, const char *cstr)
+void
+ProcessGDBRemote::DidExit ()
 {
+    // When we exit, disconnect from the GDB server communications
     m_gdb_comm.Disconnect();
-    return Process::SetExitStatus (exit_status, cstr);
 }
 
 void

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=238089&r1=238088&r2=238089&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Fri May 22 22:54:53 2015
@@ -225,10 +225,10 @@ public:
     SendEventData(const char *data) override;
 
     //----------------------------------------------------------------------
-    // Override SetExitStatus so we can disconnect from the remote GDB server
+    // Override DidExit so we can disconnect from the remote GDB server
     //----------------------------------------------------------------------
-    bool
-    SetExitStatus (int exit_status, const char *cstr) override;
+    void
+    DidExit () override;
 
     void
     SetUserSpecifiedMaxMemoryTransferSize (uint64_t user_specified_max);

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=238089&r1=238088&r2=238089&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Fri May 22 22:54:53 2015
@@ -1466,9 +1466,24 @@ Process::SetExitStatus (int status, cons
             m_exit_string.clear();
     }
 
-    DidExit ();
+    // When we exit, we no longer need to the communication channel
+    m_stdio_communication.StopReadThread();
+    m_stdio_communication.Disconnect();
+    m_stdin_forward = false;
+
+    // And we don't need the input reader anymore as well
+    if (m_process_input_reader)
+    {
+        m_process_input_reader->SetIsDone(true);
+        m_process_input_reader->Cancel();
+        m_process_input_reader.reset();
+    }
 
     SetPrivateState (eStateExited);
+
+    // Allow subclasses to do some cleanup
+    DidExit ();
+
     return true;
 }
 

Modified: lldb/trunk/source/Target/Target.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=238089&r1=238088&r2=238089&view=diff
==============================================================================
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Fri May 22 22:54:53 2015
@@ -2580,10 +2580,24 @@ Target::Launch (ProcessLaunchInfo &launc
         if (log)
             log->Printf ("Target::%s asking the platform to debug the process", __FUNCTION__);
 
+        // Get a weak pointer to the previous process if we have one
+        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 much as it can without the object being destroyed. We try to
+        // lock the shared pointer and if that works, then someone else still
+        // has a strong reference to the process.
+
+        ProcessSP old_process_sp(process_wp.lock());
+        if (old_process_sp)
+            old_process_sp->Finalize();
     }
     else
     {