[Lldb-commits] [lldb] r218537 - gtest: tweaked test runner to fix an extra comma, added more tdd-based thread coordinator behavior.

Todd Fiala todd.fiala at gmail.com
Fri Sep 26 12:08:00 PDT 2014


Author: tfiala
Date: Fri Sep 26 14:08:00 2014
New Revision: 218537

URL: http://llvm.org/viewvc/llvm-project?rev=218537&view=rev
Log:
gtest: tweaked test runner to fix an extra comma, added more tdd-based thread coordinator behavior.

Starting to flesh out the thread state coordinator class that will be used
by Linux/llgs.

Modified:
    lldb/trunk/gtest/do-gtest.py
    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/do-gtest.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/gtest/do-gtest.py?rev=218537&r1=218536&r2=218537&view=diff
==============================================================================
--- lldb/trunk/gtest/do-gtest.py (original)
+++ lldb/trunk/gtest/do-gtest.py Fri Sep 26 14:08:00 2014
@@ -41,7 +41,11 @@ def line_combine_printer(file, previous_
             return ("", 0)
 
         if len(accumulated_line) > 0:
-            new_line = accumulated_line + ", " + incoming_line
+            if accumulated_line[-2:] != ": ":
+                # Need to add a comma
+                new_line = accumulated_line + ", " + incoming_line
+            else:
+                new_line = accumulated_line + incoming_line
         else:
             new_line = incoming_line
 

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=218537&r1=218536&r2=218537&view=diff
==============================================================================
--- lldb/trunk/gtest/unittest/Plugins/Process/Linux/ThreadStateCoordinatorTest.cpp (original)
+++ lldb/trunk/gtest/unittest/Plugins/Process/Linux/ThreadStateCoordinatorTest.cpp Fri Sep 26 14:08:00 2014
@@ -22,7 +22,7 @@ TEST(ThreadStateCoordinatorTest, StopCoo
 
     coordinator.StopCoordinator ();
 
-    ASSERT_EQ(coordinator.ProcessNextEvent (), false);
+    ASSERT_EQ(false, coordinator.ProcessNextEvent ());
 }
 
 TEST(ThreadStateCoordinatorTest, CallAfterThreadsStopFiresWhenNoPendingStops)
@@ -31,19 +31,82 @@ TEST(ThreadStateCoordinatorTest, CallAft
 
     const lldb::tid_t TRIGGERING_TID = 4105;
     bool call_after_fired = false;
+    lldb::tid_t reported_firing_tid = 0;
 
     // Notify we have a trigger that needs to be fired when all threads in the wait tid set have stopped.
     coordinator.CallAfterThreadsStop (TRIGGERING_TID,
                                       EMPTY_THREAD_ID_SET,
                                       [](lldb::tid_t tid) {},
-                                      [&](lldb::tid_t tid) { call_after_fired = true; });
+                                      [&](lldb::tid_t tid) {
+                                          call_after_fired = true;
+                                          reported_firing_tid = tid;
+                                      });
 
     // Notification trigger shouldn't go off yet.
-    ASSERT_EQ (call_after_fired, false);
+    ASSERT_EQ (false, call_after_fired);
 
     // Process next event.  This will pick up the call after threads stop event.
-    ASSERT_EQ(coordinator.ProcessNextEvent (), true);
+    ASSERT_EQ (true, coordinator.ProcessNextEvent ());
 
-    // Now the trigger should have fired.
-    ASSERT_EQ(call_after_fired, true);
+    // Now the trigger should have fired, since there were no threads that needed to first stop.
+    ASSERT_EQ (true, call_after_fired);
+
+    // And the firing tid should be the one we indicated.
+    ASSERT_EQ (TRIGGERING_TID, reported_firing_tid);
+}
+
+TEST(ThreadStateCoordinatorTest, CallAfterThreadsStopFiresWhenOnePendingStop)
+{
+    ThreadStateCoordinator coordinator(NOPLogger);
+
+    const lldb::tid_t TRIGGERING_TID = 4105;
+    const lldb::tid_t PENDING_STOP_TID = 3;
+
+    ThreadStateCoordinator::ThreadIDSet pending_stop_tids { PENDING_STOP_TID };
+
+    bool call_after_fired = false;
+    lldb::tid_t reported_firing_tid = 0;
+
+    bool request_thread_stop_called = false;
+    lldb::tid_t request_thread_stop_tid = 0;
+
+    // Notify we have a trigger that needs to be fired when all threads in the wait tid set have stopped.
+    coordinator.CallAfterThreadsStop (TRIGGERING_TID,
+                                      pending_stop_tids,
+                                      [&](lldb::tid_t tid) {
+                                          request_thread_stop_called = true;
+                                          request_thread_stop_tid = tid;
+
+                                      },
+                                      [&](lldb::tid_t tid) {
+                                          call_after_fired = true;
+                                          reported_firing_tid = tid;
+                                      });
+
+    // Neither trigger should have gone off yet.
+    ASSERT_EQ (false, call_after_fired);
+    ASSERT_EQ (false, request_thread_stop_called);
+
+    // Process next event.
+    ASSERT_EQ (true, coordinator.ProcessNextEvent ());
+
+    // Now the request thread stop should have been called for the pending stop.
+    ASSERT_EQ (true, request_thread_stop_called);
+    ASSERT_EQ (PENDING_STOP_TID, request_thread_stop_tid);
+
+    // But we still shouldn't have the deferred signal call go off yet.  Need to wait for the stop to be reported.
+    ASSERT_EQ (false, call_after_fired);
+
+    // Now report the that the pending stop occurred.
+    coordinator.NotifyThreadStop (PENDING_STOP_TID);
+
+    // Shouldn't take effect until after next processing step.
+    ASSERT_EQ (false, call_after_fired);
+
+    // Process next event.
+    ASSERT_EQ (true, coordinator.ProcessNextEvent ());
+
+    // Deferred signal notification should have fired now.
+    ASSERT_EQ (true, call_after_fired);
+    ASSERT_EQ (TRIGGERING_TID, reported_firing_tid);
 }

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=218537&r1=218536&r2=218537&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp Fri Sep 26 14:08:00 2014
@@ -7,14 +7,214 @@
 //
 //===----------------------------------------------------------------------===//
 
+
+#if !defined (__STDC_FORMAT_MACROS)
+#define __STDC_FORMAT_MACROS 1
+#endif
+
+#include <inttypes.h>
+
 #include "ThreadStateCoordinator.h"
+#include <memory>
 
 using namespace lldb_private;
 
+//===----------------------------------------------------------------------===//
+
+class ThreadStateCoordinator::EventBase : public std::enable_shared_from_this<ThreadStateCoordinator::EventBase>
+{
+public:
+    EventBase ()
+    {
+    }
+
+    virtual
+    ~EventBase ()
+    {
+    }
+
+    // Return false if the coordinator should terminate running.
+    virtual bool
+    ProcessEvent (ThreadStateCoordinator &coordinator) = 0;
+};
+
+//===----------------------------------------------------------------------===//
+
+class ThreadStateCoordinator::EventStopCoordinator : public ThreadStateCoordinator::EventBase
+{
+public:
+    EventStopCoordinator ():
+        EventBase ()
+    {
+    }
+
+    ~EventStopCoordinator () override
+    {
+    }
+
+    bool
+    ProcessEvent(ThreadStateCoordinator &coordinator) override
+    {
+        return false;
+    }
+};
+
+//===----------------------------------------------------------------------===//
+
+class ThreadStateCoordinator::EventCallAfterThreadsStop : public ThreadStateCoordinator::EventBase
+{
+public:
+    EventCallAfterThreadsStop (lldb::tid_t triggering_tid,
+                               const ThreadIDSet &wait_for_stop_tids,
+                               const ThreadIDFunc &request_thread_stop_func,
+                               const ThreadIDFunc &call_after_func):
+    EventBase (),
+    m_triggering_tid (triggering_tid),
+    m_wait_for_stop_tids (wait_for_stop_tids),
+    m_request_thread_stop_func (request_thread_stop_func),
+    m_call_after_func (call_after_func)
+    {
+    }
+
+    ~EventCallAfterThreadsStop () override
+    {
+    }
+
+    lldb::tid_t GetTriggeringTID () const
+    {
+        return m_triggering_tid;
+    }
+
+    ThreadIDSet &
+    GetRemainingWaitTIDs ()
+    {
+        return m_wait_for_stop_tids;
+    }
+
+    bool
+    ProcessEvent(ThreadStateCoordinator &coordinator) override
+    {
+        // Request a stop for all the thread stops that are needed.
+        // In the future, we can keep track of which stops we're
+        // still waiting for, and can not re-issue for already
+        // requested stops.
+        for (auto tid : m_wait_for_stop_tids)
+            m_request_thread_stop_func (tid);
+
+        if (m_wait_for_stop_tids.empty ())
+        {
+        // We're not waiting for any threads.  Fire off the deferred signal delivery event.
+            NotifyNow ();
+        }
+        else
+        {
+            // The real meat of this class: wait until all
+            // the thread stops (or thread deaths) come in
+            // before firing off that the triggering signal
+            // arrived.
+            coordinator.SetPendingNotification (shared_from_this ());
+        }
+
+        return true;
+    }
+
+    void
+    NotifyNow ()
+    {
+        m_call_after_func (m_triggering_tid);
+    }
+
+private:
+
+    const lldb::tid_t m_triggering_tid;
+    ThreadIDSet m_wait_for_stop_tids;
+    ThreadIDFunc m_request_thread_stop_func;
+    ThreadIDFunc m_call_after_func;
+};
+
+//===----------------------------------------------------------------------===//
+
+class ThreadStateCoordinator::EventThreadStopped : public ThreadStateCoordinator::EventBase
+{
+public:
+    EventThreadStopped (lldb::tid_t tid):
+    EventBase (),
+    m_tid (tid)
+    {
+    }
+
+    ~EventThreadStopped () override
+    {
+    }
+
+    bool
+    ProcessEvent(ThreadStateCoordinator &coordinator) override
+    {
+        coordinator.ThreadDidStop (m_tid);
+        return true;
+    }
+
+private:
+
+    const lldb::tid_t m_tid;
+};
+
+//===----------------------------------------------------------------------===//
+
 ThreadStateCoordinator::ThreadStateCoordinator (const LogFunc &log_func) :
-    m_done_b (false),
-    m_log_func (log_func)
+    m_log_func (log_func),
+    m_event_queue (),
+    m_queue_condition (),
+    m_queue_mutex (),
+    m_tid_stop_map ()
+{
+}
+
+void
+ThreadStateCoordinator::EnqueueEvent (EventBaseSP event_sp)
+{
+    std::lock_guard<std::mutex> lock (m_queue_mutex);
+    m_event_queue.push (event_sp);
+    m_queue_condition.notify_one ();
+}
+
+ThreadStateCoordinator::EventBaseSP
+ThreadStateCoordinator::DequeueEventWithWait ()
+{
+    // Wait for an event to be present.
+    std::unique_lock<std::mutex> lock (m_queue_mutex);
+    m_queue_condition.wait (lock,
+                            [this] { return !m_event_queue.empty (); });
+
+    // Grab the event and pop it off the queue.
+    EventBaseSP event_sp = m_event_queue.front ();
+    m_event_queue.pop ();
+
+    return event_sp;
+}
+
+void
+ThreadStateCoordinator::SetPendingNotification (const EventBaseSP &event_sp)
 {
+    assert (event_sp && "null event_sp");
+    if (!event_sp)
+        return;
+
+    const EventCallAfterThreadsStop *new_call_after_event = static_cast<EventCallAfterThreadsStop*> (event_sp.get ());
+
+    if (m_pending_notification_sp)
+    {
+        const EventCallAfterThreadsStop *prev_call_after_event = static_cast<EventCallAfterThreadsStop*> (m_pending_notification_sp.get ());
+
+        // Yikes - we've already got a pending signal notification in progress.
+        // Log this info.  We lose the pending notification here.
+        Log ("ThreadStateCoordinator::%s dropping existing pending signal notification for tid %" PRIu64 ", to be replaced with signal for tid %" PRIu64,
+                   __FUNCTION__,
+                   prev_call_after_event->GetTriggeringTID (),
+                   new_call_after_event->GetTriggeringTID ());
+    }
+
+    m_pending_notification_sp = event_sp;
 }
 
 void
@@ -23,16 +223,63 @@ ThreadStateCoordinator::CallAfterThreads
                                               const ThreadIDFunc &request_thread_stop_func,
                                               const ThreadIDFunc &call_after_func)
 {
+    EnqueueEvent (EventBaseSP (new EventCallAfterThreadsStop (triggering_tid,
+                                                              wait_for_stop_tids,
+                                                              request_thread_stop_func,
+                                                              call_after_func)));
+}
+
+void
+ThreadStateCoordinator::ThreadDidStop (lldb::tid_t tid)
+{
+    // Update the global list of known thread states.  This one is definitely stopped.
+    m_tid_stop_map[tid] = true;
+
+    // If we have a pending notification, remove this from the set.
+    if (m_pending_notification_sp)
+    {
+        EventCallAfterThreadsStop *call_after_event = static_cast<EventCallAfterThreadsStop*> (m_pending_notification_sp.get ());
+
+        auto remaining_stop_tids = call_after_event->GetRemainingWaitTIDs ();
+
+        // Remove this tid if it was in it.
+        remaining_stop_tids.erase (tid);
+        if (remaining_stop_tids.empty ())
+        {
+            // Fire the pending notification now.
+            call_after_event->NotifyNow ();
+
+            // Clear the pending notification now.
+            m_pending_notification_sp.reset ();
+        }
+    }
+}
+
+void
+ThreadStateCoordinator::Log (const char *format, ...)
+{
+    va_list args;
+    va_start (args, format);
+
+    m_log_func (format, args);
+
+    va_end (args);
+}
+
+void
+ThreadStateCoordinator::NotifyThreadStop (lldb::tid_t tid)
+{
+    EnqueueEvent (EventBaseSP (new EventThreadStopped (tid)));
 }
 
 void
 ThreadStateCoordinator::StopCoordinator ()
 {
-    m_done_b = true;
+    EnqueueEvent (EventBaseSP (new EventStopCoordinator ()));
 }
 
 bool
 ThreadStateCoordinator::ProcessNextEvent ()
 {
-    return !m_done_b;
+    return DequeueEventWithWait()->ProcessEvent (*this);
 }

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=218537&r1=218536&r2=218537&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h Fri Sep 26 14:08:00 2014
@@ -10,7 +10,11 @@
 #ifndef lldb_ThreadStateCoordinator_h
 #define lldb_ThreadStateCoordinator_h
 
+#include <condition_variable>
 #include <functional>
+#include <mutex>
+#include <queue>
+#include <unordered_map>
 #include <unordered_set>
 
 #include "lldb/lldb-types.h"
@@ -24,6 +28,9 @@ namespace lldb_private
         // Typedefs.
         typedef std::unordered_set<lldb::tid_t> ThreadIDSet;
 
+        // Protocols.
+
+
         // Callback definitions.
         typedef std::function<void (lldb::tid_t tid)> ThreadIDFunc;
         typedef std::function<void (const char *format, va_list args)> LogFunc;
@@ -34,7 +41,7 @@ namespace lldb_private
         // The main purpose of the class: triggering an action after
         // a given set of threads stop.
         void
-        CallAfterThreadsStop (const lldb::tid_t triggering_tid,
+        CallAfterThreadsStop (lldb::tid_t triggering_tid,
                               const ThreadIDSet &wait_for_stop_tids,
                               const ThreadIDFunc &request_thread_stop_func,
                               const ThreadIDFunc &call_after_func);
@@ -59,26 +66,58 @@ namespace lldb_private
         // Process the next event, returning false when the coordinator is all done.
         // This call is synchronous and blocks when there are no events pending.
         // Expected usage is to run this in a separate thread until the function
-        // returns false.
+        // returns false.  Always call this from the same thread.  The processing
+        // logic assumes the execution of this is implicitly serialized.
         bool
         ProcessNextEvent ();
 
     private:
 
-        enum EventType
-        {
-            eInvalid,
-            eEventTypeCallAfterThreadsStop,
-            eEventTypeThreadStopped,
-            eEventTypeThreadResumed,
-            eEventTypeThreadCreated,
-            eEventTypeThreadDied,
-        };
+        // Typedefs.
+        class EventBase;
+
+        class EventCallAfterThreadsStop;
+        class EventStopCoordinator;
+        class EventThreadStopped;
+
+        typedef std::shared_ptr<EventBase> EventBaseSP;
+
+        typedef std::queue<EventBaseSP> QueueType;
+
+        typedef std::unordered_map<lldb::tid_t, bool> TIDBoolMap;
+
+
+        // Private member functions.
+        void
+        EnqueueEvent (EventBaseSP event_sp);
+
+        EventBaseSP
+        DequeueEventWithWait ();
 
-        bool m_done_b;
+        void
+        SetPendingNotification (const EventBaseSP &event_sp);
+
+        void
+        ThreadDidStop (lldb::tid_t tid);
 
+        void
+        Log (const char *format, ...);
+
+        // Member variables.
         LogFunc m_log_func;
 
+        QueueType m_event_queue;
+        // For now we do simple read/write lock strategy with efficient wait-for-data.
+        // We can replace with entirely non-blocking queue later but we still want the
+        // reader to sleep when nothing is available - this will be a bursty but infrequent
+        // event mechanism.
+        std::condition_variable m_queue_condition;
+        std::mutex m_queue_mutex;
+
+        EventBaseSP m_pending_notification_sp;
+
+        // Maps known TIDs to stop (true) or not-stopped (false) state.
+        TIDBoolMap m_tid_stop_map;
     };
 }
 





More information about the lldb-commits mailing list