[Lldb-commits] [lldb] r218897 - thread state coordinator: added simpler deferred stop notification method.

Todd Fiala todd.fiala at gmail.com
Thu Oct 2 12:03:07 PDT 2014


Author: tfiala
Date: Thu Oct  2 14:03:06 2014
New Revision: 218897

URL: http://llvm.org/viewvc/llvm-project?rev=218897&view=rev
Log:
thread state coordinator: added simpler deferred stop notification method.

Now that ThreadStateCoordinator errors out on threads in unexpected states,
it has enough information to know which threads need stop requests fired
when we want to do a deferred callback on a thread's behalf.  This change
adds a new method, CallAfterRunningThreadsStop(...), which no longer
takes a set of thread ids that require stop requests.  It's much harder
to misuse this method and (with newer error logic) it's harder to
correctly use the original method.  Expect the original method that takes
the set of thread ids to stop to disappear in the near future.

Adds several tests for CallAfterRunningThreadsStop().

Modified:
    lldb/trunk/gtest/unittest/Plugins/Process/Linux/ThreadStateCoordinatorTest.cpp
    lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
    lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h

Modified: lldb/trunk/gtest/unittest/Plugins/Process/Linux/ThreadStateCoordinatorTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/gtest/unittest/Plugins/Process/Linux/ThreadStateCoordinatorTest.cpp?rev=218897&r1=218896&r2=218897&view=diff
==============================================================================
--- lldb/trunk/gtest/unittest/Plugins/Process/Linux/ThreadStateCoordinatorTest.cpp (original)
+++ lldb/trunk/gtest/unittest/Plugins/Process/Linux/ThreadStateCoordinatorTest.cpp Thu Oct  2 14:03:06 2014
@@ -170,6 +170,15 @@ ASSERT_EQ (true, HasError ());
         }
 
         void
+        CallAfterRunningThreadsStop (lldb::tid_t deferred_tid)
+        {
+            m_coordinator.CallAfterRunningThreadsStop (deferred_tid,
+                                                       GetStopRequestFunction (),
+                                                       GetDeferredStopNotificationFunction (),
+                                                       GetErrorFunction ());
+        }
+
+        void
         NotifyThreadCreate (lldb::tid_t stopped_tid, bool thread_is_stopped)
         {
             m_coordinator.NotifyThreadCreate (stopped_tid, thread_is_stopped, GetErrorFunction ());
@@ -671,3 +680,94 @@ TEST_F (ThreadStateCoordinatorTest, Resu
     ASSERT_EQ (true, DidFireDeferredNotification ());
     ASSERT_EQ (TRIGGERING_TID, GetDeferredNotificationTID ());
 }
+
+TEST_F (ThreadStateCoordinatorTest, CallAfterRunningThreadsStopFiresWhenNoRunningThreads)
+{
+    // Let the coordinator know about our thread.
+    SetupKnownStoppedThread (TRIGGERING_TID);
+
+    // Notify we have a trigger that needs to be fired when all running threads have stopped.
+    CallAfterRunningThreadsStop (TRIGGERING_TID);
+
+    // Notification trigger shouldn't go off yet.
+    ASSERT_EQ (false, DidFireDeferredNotification ());
+
+    // Process next event.  This will pick up the call after threads stop event.
+    ASSERT_PROCESS_NEXT_EVENT_SUCCEEDS ();
+
+    // Now the trigger should have fired, since there were no threads that needed to first stop.
+    ASSERT_EQ (true, DidFireDeferredNotification ());
+    ASSERT_EQ (TRIGGERING_TID, GetDeferredNotificationTID ());
+
+    // And no stop requests should have been made.
+    ASSERT_EQ (0, GetRequestedStopCount ());
+}
+
+TEST_F (ThreadStateCoordinatorTest, CallAfterRunningThreadsStopRequestsTwoPendingStops)
+{
+    // Let the coordinator know about our threads.
+    SetupKnownStoppedThread (TRIGGERING_TID);
+    SetupKnownRunningThread (PENDING_STOP_TID);
+    SetupKnownRunningThread (PENDING_STOP_TID_02);
+
+    // Notify we have a trigger that needs to be fired when all running threads have stopped.
+    CallAfterRunningThreadsStop (TRIGGERING_TID);
+
+    // Notification trigger shouldn't go off yet.
+    ASSERT_EQ (false, DidFireDeferredNotification ());
+
+    // Process next event.  This will pick up the call after threads stop event.
+    ASSERT_PROCESS_NEXT_EVENT_SUCCEEDS ();
+
+    // We should have two stop requests for the two threads currently running.
+    ASSERT_EQ (2, GetRequestedStopCount ());
+    ASSERT_EQ (true, DidRequestStopForTid (PENDING_STOP_TID));
+    ASSERT_EQ (true, DidRequestStopForTid (PENDING_STOP_TID_02));
+
+    // But the deferred stop notification should not have fired yet.
+    ASSERT_EQ (false, DidFireDeferredNotification ());
+
+    // Now notify the two threads stopped.
+    NotifyThreadStop (PENDING_STOP_TID);
+    ASSERT_PROCESS_NEXT_EVENT_SUCCEEDS ();
+    ASSERT_EQ (false, DidFireDeferredNotification ());
+
+    NotifyThreadStop (PENDING_STOP_TID_02);
+    ASSERT_PROCESS_NEXT_EVENT_SUCCEEDS ();
+
+    // Now the trigger should have fired, since there were no threads that needed to first stop.
+    ASSERT_EQ (true, DidFireDeferredNotification ());
+    ASSERT_EQ (TRIGGERING_TID, GetDeferredNotificationTID ());
+}
+
+TEST_F (ThreadStateCoordinatorTest, CallAfterRunningThreadsStopRequestsStopTwoOtherThreadsOneRunning)
+{
+    // Let the coordinator know about our threads.  PENDING_STOP_TID_02 will already be stopped.
+    SetupKnownStoppedThread (TRIGGERING_TID);
+    SetupKnownRunningThread (PENDING_STOP_TID);
+    SetupKnownStoppedThread (PENDING_STOP_TID_02);
+
+    // Notify we have a trigger that needs to be fired when all running threads have stopped.
+    CallAfterRunningThreadsStop (TRIGGERING_TID);
+
+    // Notification trigger shouldn't go off yet.
+    ASSERT_EQ (false, DidFireDeferredNotification ());
+
+    // Process next event.  This will pick up the call after threads stop event.
+    ASSERT_PROCESS_NEXT_EVENT_SUCCEEDS ();
+
+    // We should have two stop requests for the two threads currently running.
+    ASSERT_EQ (1, GetRequestedStopCount ());
+    ASSERT_EQ (true, DidRequestStopForTid (PENDING_STOP_TID));
+
+    // But the deferred stop notification should not have fired yet.
+    ASSERT_EQ (false, DidFireDeferredNotification ());
+
+    // Now notify the two threads stopped.
+    NotifyThreadStop (PENDING_STOP_TID);
+    ASSERT_PROCESS_NEXT_EVENT_SUCCEEDS ();
+
+    // Now the trigger should have fired, since there were no threads that needed to first stop.
+    ASSERT_EQ (true, DidFireDeferredNotification ());
+    ASSERT_EQ (TRIGGERING_TID, GetDeferredNotificationTID ());
+}

Modified: lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp?rev=218897&r1=218896&r2=218897&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp Thu Oct  2 14:03:06 2014
@@ -73,7 +73,23 @@ public:
     m_original_wait_for_stop_tids (wait_for_stop_tids),
     m_request_thread_stop_function (request_thread_stop_function),
     m_call_after_function (call_after_function),
-    m_error_function (error_function)
+    m_error_function (error_function),
+    m_request_stop_on_all_unstopped_threads (false)
+    {
+    }
+
+    EventCallAfterThreadsStop (lldb::tid_t triggering_tid,
+                               const ThreadIDFunction &request_thread_stop_function,
+                               const ThreadIDFunction &call_after_function,
+                               const ErrorFunction &error_function) :
+    EventBase (),
+    m_triggering_tid (triggering_tid),
+    m_wait_for_stop_tids (),
+    m_original_wait_for_stop_tids (),
+    m_request_thread_stop_function (request_thread_stop_function),
+    m_call_after_function (call_after_function),
+    m_error_function (error_function),
+    m_request_stop_on_all_unstopped_threads (true)
     {
     }
 
@@ -116,42 +132,16 @@ public:
             return eventLoopResultContinue;
         }
 
-        // Request a stop for all the thread stops that need to be stopped
-        // and are not already known to be stopped.  Keep a list of all the
-        // threads from which we still need to hear a stop reply.
-
-        ThreadIDSet sent_tids;
-        for (auto tid : m_wait_for_stop_tids)
+        if (m_request_stop_on_all_unstopped_threads)
         {
-            // Validate we know about all tids for which we must first receive a stop before
-            // triggering the deferred stop notification.
-            auto find_it = coordinator.m_tid_stop_map.find (tid);
-            if (find_it == coordinator.m_tid_stop_map.end ())
-            {
-                // This is an error.  We shouldn't be asking for waiting pids that aren't known.
-                // NOTE: we may be stripping out the specification of wait tids and handle this
-                // automatically, in which case this state can never occur.
-                std::ostringstream error_message;
-                error_message << "error: deferred notification for tid " << m_triggering_tid << " specified an unknown/untracked pending stop tid " << m_triggering_tid;
-                m_error_function (error_message.str ());
-
-                // Bail out here.
+            RequestStopOnAllRunningThreads (coordinator);
+        }
+        else
+        {
+            if (!RequestStopOnAllSpecifiedThreads (coordinator))
                 return eventLoopResultContinue;
-            }
-
-            // If the pending stop thread is currently running, we need to send it a stop request.
-            if (!find_it->second)
-            {
-                m_request_thread_stop_function (tid);
-                sent_tids.insert (tid);
-            }
         }
 
-        // We only need to wait for the sent_tids - so swap our wait set
-        // to the sent tids.  The rest are already stopped and we won't
-        // be receiving stop notifications for them.
-        m_wait_for_stop_tids.swap (sent_tids);
-
         if (m_wait_for_stop_tids.empty ())
         {
         // We're not waiting for any threads.  Fire off the deferred signal delivery event.
@@ -208,12 +198,81 @@ private:
         m_call_after_function (m_triggering_tid);
     }
 
+    bool
+    RequestStopOnAllSpecifiedThreads (const ThreadStateCoordinator &coordinator)
+    {
+        // Request a stop for all the thread stops that need to be stopped
+        // and are not already known to be stopped.  Keep a list of all the
+        // threads from which we still need to hear a stop reply.
+
+        ThreadIDSet sent_tids;
+        for (auto tid : m_wait_for_stop_tids)
+        {
+            // Validate we know about all tids for which we must first receive a stop before
+            // triggering the deferred stop notification.
+            auto find_it = coordinator.m_tid_stop_map.find (tid);
+            if (find_it == coordinator.m_tid_stop_map.end ())
+            {
+                // This is an error.  We shouldn't be asking for waiting pids that aren't known.
+                // NOTE: we may be stripping out the specification of wait tids and handle this
+                // automatically, in which case this state can never occur.
+                std::ostringstream error_message;
+                error_message << "error: deferred notification for tid " << m_triggering_tid << " specified an unknown/untracked pending stop tid " << m_triggering_tid;
+                m_error_function (error_message.str ());
+
+                // Bail out here.
+                return false;
+            }
+
+            // If the pending stop thread is currently running, we need to send it a stop request.
+            if (!find_it->second)
+            {
+                m_request_thread_stop_function (tid);
+                sent_tids.insert (tid);
+            }
+        }
+
+        // We only need to wait for the sent_tids - so swap our wait set
+        // to the sent tids.  The rest are already stopped and we won't
+        // be receiving stop notifications for them.
+        m_wait_for_stop_tids.swap (sent_tids);
+
+        // Succeeded, keep running.
+        return true;
+    }
+
+    void
+    RequestStopOnAllRunningThreads (const ThreadStateCoordinator &coordinator)
+    {
+        // Request a stop for all the thread stops that need to be stopped
+        // and are not already known to be stopped.  Keep a list of all the
+        // threads from which we still need to hear a stop reply.
+
+        ThreadIDSet sent_tids;
+        for (auto it = coordinator.m_tid_stop_map.begin(); it != coordinator.m_tid_stop_map.end(); ++it)
+        {
+            // We only care about threads not stopped.
+            const bool running = !it->second;
+            if (running)
+            {
+                // Request this thread stop.
+                const lldb::tid_t tid = it->first;
+                m_request_thread_stop_function (tid);
+                sent_tids.insert (tid);
+            }
+        }
+
+        // Set the wait list to the set of tids for which we requested stops.
+        m_wait_for_stop_tids.swap (sent_tids);
+    }
+
     const lldb::tid_t m_triggering_tid;
     ThreadIDSet m_wait_for_stop_tids;
     const ThreadIDSet m_original_wait_for_stop_tids;
     ThreadIDFunction m_request_thread_stop_function;
     ThreadIDFunction m_call_after_function;
     ErrorFunction m_error_function;
+    const bool m_request_stop_on_all_unstopped_threads;
 };
 
 //===----------------------------------------------------------------------===//
@@ -464,6 +523,18 @@ ThreadStateCoordinator::CallAfterThreads
                                                               request_thread_stop_function,
                                                               call_after_function,
                                                               error_function)));
+}
+
+void
+ThreadStateCoordinator::CallAfterRunningThreadsStop (const lldb::tid_t triggering_tid,
+                                                     const ThreadIDFunction &request_thread_stop_function,
+                                                     const ThreadIDFunction &call_after_function,
+                                                     const ErrorFunction &error_function)
+{
+    EnqueueEvent (EventBaseSP (new EventCallAfterThreadsStop (triggering_tid,
+                                                              request_thread_stop_function,
+                                                              call_after_function,
+                                                              error_function)));
 }
 
 void

Modified: lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h?rev=218897&r1=218896&r2=218897&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h Thu Oct  2 14:03:06 2014
@@ -71,6 +71,16 @@ namespace lldb_private
                               const ThreadIDFunction &call_after_function,
                               const ErrorFunction &error_function);
 
+        // This method is the main purpose of the class: triggering a deferred
+        // action after all non-stopped threads stop.  The triggering_tid is the
+        // thread id passed to the call_after_function.  The error_function will
+        // be fired if the triggering tid is unknown at the time of execution.
+        void
+        CallAfterRunningThreadsStop (lldb::tid_t triggering_tid,
+                                     const ThreadIDFunction &request_thread_stop_function,
+                                     const ThreadIDFunction &call_after_function,
+                                     const ErrorFunction &error_function);
+
         // Notify the thread stopped.  Will trigger error at time of execution if we
         // already think it is stopped.
         void





More information about the lldb-commits mailing list