Fix some long standing issues that caused tests to be flaky.

The main issues were:
- Listeners recently were converted over to used by getting a shared pointer to a listener. And when they listened to broadcasters they would get a strong reference added to them meaning the listeners would never go away. This caused memory usage to increase and would cause performance issue if many steps were done.
- The lldb_private::Process private state thread had an issue where if a "stop" contol signal was attempted to be sent to that thread, it could end up not responding in 2 seconds and end up getting cancelled which might cause us to cancel a thread that had a mutex locked and it would deadlock the test.

This change makes broadcasters hold onto weak references to listeners. It also fixes some bad threading code that had races inside of it by making the m_events_mutex be non-recursive and getting rid of fragile use of a Predicate<bool> to say that new events are available, and replacing it with using the m_events_mutex with a new m_events_condition to control access to the events in a safer way.

The private state thread now uses a safer way to communicate that the control event has been received by the private state thread: it makes a EventDataReceipt instance that it attaches to the event that sends the control to the private state thread and used this to synchronize the fact that the private state thread has received the event instead of using a Predicate<bool> to convey the info. When the signal event is received, it will pull the event off of the queue in the private state thread and cause the EventData::DoOnRemoval() to be called, which will signal that the event has been received. This cleans up the signal delivery notification so it doesn't rely on a member variable of the process class to convey the info.

std::shared_ptr<EventDataReceipt> event_receipt_sp(new EventDataReceipt());
m_private_state_control_broadcaster.BroadcastEvent(signal, event_receipt_sp);

<rdar://problem/26256353> Listeners are being kept around longer than they should be due to recent changs
<rdar://problem/26256258> Private process state thread can be cancelled and cause deadlocks in test suite

llvm-svn: 269377

1 files changed, 33 insertions, 35 deletions

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index f156d086d28..a880782663a 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -726,7 +726,6 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp, const UnixSig
     m_private_state_broadcaster(nullptr, "lldb.process.internal_state_broadcaster"),
     m_private_state_control_broadcaster(nullptr, "lldb.process.internal_state_control_broadcaster"),
     m_private_state_listener_sp (Listener::MakeListener("lldb.process.internal_state_listener")),
-    m_private_state_control_wait(),
     m_mod_id (),
     m_process_unique_id(0),
     m_thread_index_id (0),
@@ -4109,44 +4108,46 @@ Process::ControlPrivateStateThread (uint32_t signal)
     // Signal the private state thread. First we should copy this is case the
     // thread starts exiting since the private state thread will NULL this out
     // when it exits
-    HostThread private_state_thread(m_private_state_thread);
-    if (private_state_thread.IsJoinable())
     {
-        TimeValue timeout_time;
-        bool timed_out;
-
-        m_private_state_control_broadcaster.BroadcastEvent(signal, nullptr);
-
-        timeout_time = TimeValue::Now();
-        timeout_time.OffsetWithSeconds(2);
-        if (log)
-            log->Printf ("Sending control event of type: %d.", signal);
-        m_private_state_control_wait.WaitForValueEqualTo (true, &timeout_time, &timed_out);
-        m_private_state_control_wait.SetValue (false, eBroadcastNever);
-
-        if (signal == eBroadcastInternalStateControlStop)
+        HostThread private_state_thread(m_private_state_thread);
+        if (private_state_thread.IsJoinable())
         {
-            if (timed_out)
+            if (log)
+                log->Printf ("Sending control event of type: %d.", signal);
+            // Send the control event and wait for the receipt or for the private state
+            // thread to exit
+            std::shared_ptr<EventDataReceipt> event_receipt_sp(new EventDataReceipt());
+            m_private_state_control_broadcaster.BroadcastEvent(signal, event_receipt_sp);
+
+            bool receipt_received = false;
+            while (!receipt_received)
             {
-                Error error = private_state_thread.Cancel();
-                if (log)
-                    log->Printf ("Timed out responding to the control event, cancel got error: \"%s\".", error.AsCString());
+                bool timed_out = false;
+                TimeValue timeout_time;
+                timeout_time = TimeValue::Now();
+                timeout_time.OffsetWithSeconds(2);
+                // Check for a receipt for 2 seconds and then check if the private state
+                // thread is still around.
+                receipt_received = event_receipt_sp->WaitForEventReceived (&timeout_time, &timed_out);
+                if (!receipt_received)
+                {
+                    // Check if the private state thread is still around. If it isn't then we are done waiting
+                    if (!m_private_state_thread.IsJoinable())
+                        break; // Private state thread exited, we are done
+                }
             }
-            else
+
+            if (signal == eBroadcastInternalStateControlStop)
             {
-                if (log)
-                    log->Printf ("The control event killed the private state thread without having to cancel.");
+                thread_result_t result = NULL;
+                private_state_thread.Join(&result);
             }
-
-            thread_result_t result = NULL;
-            private_state_thread.Join(&result);
-            m_private_state_thread.Reset();
         }
-    }
-    else
-    {
-        if (log)
-            log->Printf ("Private state thread already dead, no need to signal it to stop.");
+        else
+        {
+            if (log)
+                log->Printf ("Private state thread already dead, no need to signal it to stop.");
+        }
     }
 }
 
@@ -4311,7 +4312,6 @@ thread_result_t
 Process::RunPrivateStateThread (bool is_secondary_thread)
 {
     bool control_only = true;
-    m_private_state_control_wait.SetValue (false, eBroadcastNever);
 
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     if (log)
@@ -4346,7 +4346,6 @@ Process::RunPrivateStateThread (bool is_secondary_thread)
                 break;
             }
 
-            m_private_state_control_wait.SetValue (true, eBroadcastAlways);
             continue;
         }
         else if (event_sp->GetType() == eBroadcastBitInterrupt)
@@ -4442,7 +4441,6 @@ Process::RunPrivateStateThread (bool is_secondary_thread)
     // try to change it on the way out.
     if (!is_secondary_thread)
         m_public_run_lock.SetStopped();
-    m_private_state_control_wait.SetValue (true, eBroadcastAlways);
     m_private_state_thread.Reset();
     return NULL;
 }