[Lldb-commits] [PATCH] Fix deadlock in Listener/Broadcaster::Clear

[Pavel Labath]

Hi clayborg,

When calling Listener::Clear() and Broadcaster::Clear() simultaneously from two threads a
deadlock would occur. Each object would obtain its internal lock (m_listeners_mutex,
m_broadcasters_mutex) and then proceed to deregister itself from the other object by calling
Broadcaster::RemoveListener() and Listener::BroadcasterWillDestruct() respectively. These
functions would attempt to obtain these locks again, resulting in a deadlock.

I resolve this issue my moving the lists of broadcasters and listeners to a local variable in the
Clear() function, so that RemoveListener() and BroadcasterWillDestruct() can be called without
holding the internal lock. Strictly speaking, doing this in only one function would be
sufficient, but I modified both for added safety.

http://reviews.llvm.org/D8211

Files:
  source/Core/Broadcaster.cpp
  source/Core/Listener.cpp

Index: source/Core/Broadcaster.cpp
===================================================================
--- source/Core/Broadcaster.cpp
+++ source/Core/Broadcaster.cpp
@@ -57,17 +57,21 @@
 void
 Broadcaster::Clear()
 {
-    Mutex::Locker listeners_locker(m_listeners_mutex);
+    collection old_listeners;
+    {
+        Mutex::Locker listeners_locker(m_listeners_mutex);
+        // Move the old listeners out of the way, so we can process them without holding the
+        // mutex.
+        old_listeners.swap(m_listeners);
+    }
     
     // Make sure the listener forgets about this broadcaster. We do
     // this in the broadcaster in case the broadcaster object initiates
     // the removal.
     
-    collection::iterator pos, end = m_listeners.end();
-    for (pos = m_listeners.begin(); pos != end; ++pos)
+    collection::iterator pos, end = old_listeners.end();
+    for (pos = old_listeners.begin(); pos != end; ++pos)
         pos->first->BroadcasterWillDestruct (this);
-    
-    m_listeners.clear();
 }
 const ConstString &
 Broadcaster::GetBroadcasterName ()
Index: source/Core/Listener.cpp
===================================================================
--- source/Core/Listener.cpp
+++ source/Core/Listener.cpp
@@ -57,15 +57,20 @@
 void
 Listener::Clear()
 {
-    Mutex::Locker locker(m_broadcasters_mutex);
-    broadcaster_collection::iterator pos, end = m_broadcasters.end();
-    for (pos = m_broadcasters.begin(); pos != end; ++pos)
+    broadcaster_collection old_broadcasters;
+    {
+        Mutex::Locker locker(m_broadcasters_mutex);
+        // Move the old broadcasters out of the way, so we can process them without holding the
+        // mutex.
+        old_broadcasters.swap(m_broadcasters);
+        m_cond_wait.SetValue (false, eBroadcastNever);
+        Mutex::Locker event_locker(m_events_mutex);
+        m_events.clear();
+    }
+
+    broadcaster_collection::iterator pos, end = old_broadcasters.end();
+    for (pos = old_broadcasters.begin(); pos != end; ++pos)
         pos->first->RemoveListener (this, pos->second.event_mask);
-    m_broadcasters.clear();
-    m_cond_wait.SetValue (false, eBroadcastNever);
-    m_broadcasters.clear();
-    Mutex::Locker event_locker(m_events_mutex);
-    m_events.clear();
 }
 
 uint32_t

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D8211.21600.patch
Type: text/x-patch
Size: 2281 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150310/24ed075c/attachment.bin>