[Lldb-commits] [lldb] r167857 - in /lldb/trunk: include/lldb/Core/Listener.h source/Core/Listener.cpp

[Jim Ingham]

Author: jingham
Date: Tue Nov 13 13:09:45 2012
New Revision: 167857

URL: http://llvm.org/viewvc/llvm-project?rev=167857&view=rev
Log:
Revision of the patch from Andrew Kaylor <andrew.kaylor at intel.com> to prevent missing an event added in WaitForEventsInternal, narrowing the time in which we are not accepting new events.  Also, made everything that was protected private, since there really isn't any good reason why subclasses would have to muck with the listener internals.

Modified:
    lldb/trunk/include/lldb/Core/Listener.h
    lldb/trunk/source/Core/Listener.cpp

Modified: lldb/trunk/include/lldb/Core/Listener.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Listener.h?rev=167857&r1=167856&r2=167857&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Core/Listener.h (original)
+++ lldb/trunk/include/lldb/Core/Listener.h Tue Nov 13 13:09:45 2012
@@ -117,7 +117,7 @@
     size_t
     HandleBroadcastEvent (lldb::EventSP &event_sp);
 
-protected:
+private:
 
     //------------------------------------------------------------------
     // Classes that inherit from Listener can see and modify these
@@ -177,7 +177,6 @@
     void
     BroadcasterManagerWillDestruct (BroadcasterManager *manager);
     
-private:
 
 //    broadcaster_collection::iterator
 //    FindBroadcasterWithMask (Broadcaster *broadcaster,

Modified: lldb/trunk/source/Core/Listener.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Listener.cpp?rev=167857&r1=167856&r2=167857&view=diff
==============================================================================
--- lldb/trunk/source/Core/Listener.cpp (original)
+++ lldb/trunk/source/Core/Listener.cpp Tue Nov 13 13:09:45 2012
@@ -312,6 +312,11 @@
                 m_cond_wait.SetValue (false, eBroadcastNever);
         }
         
+        // Unlock the event queue here.  We've removed this event and are about to return
+        // it so it should be okay to get the next event off the queue here - and it might
+        // be useful to do that in the "DoOnRemoval".
+        lock.Unlock();
+        
         // Don't call DoOnRemoval if you aren't removing the event...
         if (remove)
             event_sp->DoOnRemoval();
@@ -406,19 +411,23 @@
 
     while (1)
     {
-        // Scope for "event_locker"
-        {
-            // Don't allow new events to be added while we're checking for the
-            // one we want.  Otherwise, setting m_cond_wait to false below
-            // might cause us to miss one.
-            Mutex::Locker event_locker(m_events_mutex);
-
-            if (GetNextEventInternal (broadcaster, broadcaster_names, num_broadcaster_names, event_type_mask, event_sp))
+        // Note, we don't want to lock the m_events_mutex in the call to GetNextEventInternal, since the DoOnRemoval
+        // code might require that new events be serviced.  For instance, the Breakpoint Command's 
+        if (GetNextEventInternal (broadcaster, broadcaster_names, num_broadcaster_names, event_type_mask, event_sp))
                 return true;
 
+        {
             // Reset condition value to false, so we can wait for new events to be
             // added that might meet our current filter
-            m_cond_wait.SetValue (false, eBroadcastNever);
+            // But first poll for any new event that might satisfy our condition, and if so consume it,
+            // otherwise wait.
+            
+            Mutex::Locker event_locker(m_events_mutex);
+            const bool remove = false;
+            if (FindNextEventInternal (broadcaster, broadcaster_names, num_broadcaster_names, event_type_mask, event_sp, remove))
+                continue;
+            else
+                m_cond_wait.SetValue (false, eBroadcastNever);
         }
 
         if (m_cond_wait.WaitForValueEqualTo (true, timeout, &timed_out))