[Lldb-commits] [lldb] 9321255 - [lldb/Core] Avoid more Communication::Disconnect races

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2020-04-23T16:36:21+02:00
New Revision: 9321255b8825d9165b2884204348a353af36d212

URL: https://github.com/llvm/llvm-project/commit/9321255b8825d9165b2884204348a353af36d212
DIFF: https://github.com/llvm/llvm-project/commit/9321255b8825d9165b2884204348a353af36d212.diff

LOG: [lldb/Core] Avoid more Communication::Disconnect races

Calling Disconnect while the read thread is running is racy because the
thread can also call Disconnect.  This is a follow-up to b424b0bf, which
reorders other occurences of Disconnect/StopReadThread I can find, and also
adds an assertion to guard against new occurences being introduced.

Added: 
    

Modified: 
    lldb/source/Core/Communication.cpp
    lldb/source/Target/Process.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Core/Communication.cpp b/lldb/source/Core/Communication.cpp
index 2990f4157584..b358e70b1a91 100644
--- a/lldb/source/Core/Communication.cpp
+++ b/lldb/source/Core/Communication.cpp
@@ -71,8 +71,8 @@ Communication::~Communication() {
 
 void Communication::Clear() {
   SetReadThreadBytesReceivedCallback(nullptr, nullptr);
-  Disconnect(nullptr);
   StopReadThread(nullptr);
+  Disconnect(nullptr);
 }
 
 ConnectionStatus Communication::Connect(const char *url, Status *error_ptr) {
@@ -93,6 +93,8 @@ ConnectionStatus Communication::Disconnect(Status *error_ptr) {
   LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMUNICATION),
            "{0} Communication::Disconnect ()", this);
 
+  assert((!m_read_thread_enabled || m_read_thread_did_exit) &&
+         "Disconnecting while the read thread is running is racy!");
   lldb::ConnectionSP connection_sp(m_connection_sp);
   if (connection_sp) {
     ConnectionStatus status = connection_sp->Disconnect(error_ptr);

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 45dcfc489d81..989cdae8b402 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3316,8 +3316,8 @@ Status Process::Destroy(bool force_kill) {
       DidDestroy();
       StopPrivateStateThread();
     }
-    m_stdio_communication.Disconnect();
     m_stdio_communication.StopReadThread();
+    m_stdio_communication.Disconnect();
     m_stdin_forward = false;
 
     if (m_process_input_reader) {