Bug 1202497 - part 6 - make the locking requirements of nsEventQueue explicit; r=gerald

Like the previous patch, this patch is a no-op change in terms of
functionality.  It does, however, pave part of the way for forcing
clients of nsEventQueue to provide their own locking.

netwerk/base/nsSocketTransportService2.cpp

@@ -100,6 +100,7 @@ nsSocketTransportService::nsSocketTransportService()
     , mIdleCount(0)
     , mSentBytesCount(0)
     , mReceivedBytesCount(0)
+    , mEventQueueLock("nsSocketTransportService::mEventQueueLock")
     , mSendBufferSize(0)
     , mKeepaliveIdleTimeS(600)
     , mKeepaliveRetryIntervalS(1)
@@ -198,7 +199,10 @@ nsSocketTransportService::NotifyWhenCanAttachSocket(nsIRunnable *event)
         return Dispatch(event, NS_DISPATCH_NORMAL);
     }
 
-    mPendingSocketQ.PutEvent(event);
+    {
+      MutexAutoLock lock(mEventQueueLock);
+      mPendingSocketQ.PutEvent(event, lock);
+    }
     return NS_OK;
 }
 
@@ -251,7 +255,11 @@ nsSocketTransportService::DetachSocket(SocketContext *listHead, SocketContext *s
     // notify the first element on the pending socket queue...
     //
     nsCOMPtr<nsIRunnable> event;
-    if (mPendingSocketQ.GetPendingEvent(getter_AddRefs(event))) {
+    {
+      MutexAutoLock lock(mEventQueueLock);
+      mPendingSocketQ.GetPendingEvent(getter_AddRefs(event), lock);
+    }
+    if (event) {
         // move event from pending queue to dispatch queue
         return Dispatch(event, NS_DISPATCH_NORMAL);
     }

netwerk/base/nsSocketTransportService2.h

@@ -222,6 +222,7 @@ private:
     // pending socket queue - see NotifyWhenCanAttachSocket
     //-------------------------------------------------------------------------
 
+    mozilla::Mutex mEventQueueLock;
     nsEventQueue mPendingSocketQ; // queue of nsIRunnable objects
 
     // Preference Monitor for SendBufferSize and Keepalive prefs.

xpcom/threads/nsEventQueue.cpp

@@ -61,7 +61,8 @@ template nsEventQueueBase<Monitor>::~nsEventQueueBase();
 template<typename MonitorType>
 bool
 nsEventQueueBase<MonitorType>::GetEvent(bool aMayWait, nsIRunnable** aResult,
-                                        MonitorAutoEnterType& aProofOfLock)
+                                        MonitorAutoEnterType& aProofOfLock,
+                                        MutexAutoLock&)
 {
   while (IsEmpty()) {
     if (!aMayWait) {
@@ -91,21 +92,24 @@ nsEventQueueBase<MonitorType>::GetEvent(bool aMayWait, nsIRunnable** aResult,
 }
 
 template bool nsEventQueueBase<Monitor>::GetEvent(bool aMayWait, nsIRunnable** aResult,
-                                                  MonitorAutoLock& aProofOfLock);
+                                                  MonitorAutoLock& aProofOfLock,
+                                                  MutexAutoLock&);
 
 bool
-nsEventQueue::GetEvent(bool aMayWait, nsIRunnable** aEvent)
+nsEventQueue::GetEvent(bool aMayWait, nsIRunnable** aEvent,
+                       MutexAutoLock& aExtraneousLock)
 {
   MonitorAutoEnterType mon(mMonitor);
 
-  return Base::GetEvent(aMayWait, aEvent, mon);
+  return Base::GetEvent(aMayWait, aEvent, mon, aExtraneousLock);
 }
 
 template<typename MonitorType>
 void
 nsEventQueueBase<MonitorType>::PutEvent(
     already_AddRefed<nsIRunnable>&& aRunnable,
-    MonitorAutoEnterType& aProofOfLock)
+    MonitorAutoEnterType& aProofOfLock,
+    MutexAutoLock&)
 {
   if (!mHead) {
     mHead = NewPage();
@@ -132,17 +136,19 @@ nsEventQueueBase<MonitorType>::PutEvent(
 }
 
 template void nsEventQueueBase<Monitor>::PutEvent(already_AddRefed<nsIRunnable>&& aRunnable,
-                                                  MonitorAutoLock& aProofOfLock);
+                                                  MonitorAutoLock& aProofOfLock,
+                                                  MutexAutoLock&);
 
 void
-nsEventQueue::PutEvent(nsIRunnable* aRunnable)
+nsEventQueue::PutEvent(nsIRunnable* aRunnable, MutexAutoLock& aExtraneousLock)
 {
   nsCOMPtr<nsIRunnable> event(aRunnable);
-  PutEvent(event.forget());
+  PutEvent(event.forget(), aExtraneousLock);
 }
 
 void
-nsEventQueue::PutEvent(already_AddRefed<nsIRunnable>&& aRunnable)
+nsEventQueue::PutEvent(already_AddRefed<nsIRunnable>&& aRunnable,
+                       MutexAutoLock& aExtraneousLock)
 {
   if (ChaosMode::isActive(ChaosFeature::ThreadScheduling)) {
     // With probability 0.5, yield so other threads have a chance to
@@ -154,12 +160,13 @@ nsEventQueue::PutEvent(already_AddRefed<nsIRunnable>&& aRunnable)
 
   MonitorAutoEnterType mon(mMonitor);
 
-  Base::PutEvent(Move(aRunnable), mon);
+  Base::PutEvent(Move(aRunnable), mon, aExtraneousLock);
 }
 
 template<typename MonitorType>
 size_t
-nsEventQueueBase<MonitorType>::Count(MonitorAutoEnterType& aProofOfLock)
+nsEventQueueBase<MonitorType>::Count(MonitorAutoEnterType& aProofOfLock,
+                                     MutexAutoLock&)
 {
   // It is obvious count is 0 when the queue is empty.
   if (!mHead) {
@@ -192,12 +199,13 @@ nsEventQueueBase<MonitorType>::Count(MonitorAutoEnterType& aProofOfLock)
   return count;
 }
 
-template size_t nsEventQueueBase<Monitor>::Count(MonitorAutoLock& aProofOfLock);
+template size_t nsEventQueueBase<Monitor>::Count(MonitorAutoLock& aProofOfLock,
+                                                 MutexAutoLock&);
 
 size_t
-nsEventQueue::Count()
+nsEventQueue::Count(MutexAutoLock& aExtraneousLock)
 {
   MonitorAutoEnterType mon(mMonitor);
 
-  return Base::Count(mon);
+  return Base::Count(mon, aExtraneousLock);
 }

xpcom/threads/nsEventQueue.h

@@ -26,6 +26,8 @@ struct MonitorAutoEnterChooser<mozilla::Monitor>
 template<typename MonitorType>
 class nsEventQueueBase
 {
+  typedef mozilla::MutexAutoLock MutexAutoLock;
+
 public:
   typedef MonitorType Monitor;
   typedef typename MonitorAutoEnterChooser<Monitor>::Type MonitorAutoEnterType;
@@ -37,7 +39,7 @@ public:
   // a strong reference to the event after this method returns.  This method
   // cannot fail.
   void PutEvent(already_AddRefed<nsIRunnable>&& aEvent,
-                MonitorAutoEnterType& aProofOfLock);
+                MonitorAutoEnterType& aProofOfLock, MutexAutoLock&);
 
   // This method gets an event from the event queue.  If mayWait is true, then
   // the method will block the calling thread until an event is available.  If
@@ -46,16 +48,16 @@ public:
   // caller is responsible for releasing the event object.  This method does
   // not alter the reference count of the resulting event.
   bool GetEvent(bool aMayWait, nsIRunnable** aEvent,
-                MonitorAutoEnterType& aProofOfLock);
+                MonitorAutoEnterType& aProofOfLock, MutexAutoLock&);
 
   // This method returns the next pending event or null.
   bool GetPendingEvent(nsIRunnable** aRunnable,
-                       MonitorAutoEnterType& aProofOfLock)
+                       MonitorAutoEnterType& aProofOfLock, MutexAutoLock& aExtraneousLock)
   {
-    return GetEvent(false, aRunnable, aProofOfLock);
+    return GetEvent(false, aRunnable, aProofOfLock, aExtraneousLock);
   }
 
-  size_t Count(MonitorAutoEnterType& aProofOfLock);
+  size_t Count(MonitorAutoEnterType& aProofOfLock, MutexAutoLock&);
 
 private:
   bool IsEmpty()
@@ -103,6 +105,7 @@ private:
   // Can't use typedefs or type alias templates here to name the base type.
   friend class nsEventQueueBase<mozilla::Monitor>;
 
+  typedef mozilla::MutexAutoLock MutexAutoLock;
   typedef Base::Monitor MonitorType;
   typedef Base::MonitorAutoEnterType MonitorAutoEnterType;
   MonitorType mMonitor;
@@ -113,8 +116,8 @@ public:
   // This method adds a new event to the pending event queue.  The queue holds
   // a strong reference to the event after this method returns.  This method
   // cannot fail.
-  void PutEvent(nsIRunnable* aEvent);
-  void PutEvent(already_AddRefed<nsIRunnable>&& aEvent);
+  void PutEvent(nsIRunnable* aEvent, MutexAutoLock&);
+  void PutEvent(already_AddRefed<nsIRunnable>&& aEvent, MutexAutoLock&);
 
   // This method gets an event from the event queue.  If mayWait is true, then
   // the method will block the calling thread until an event is available.  If
@@ -122,21 +125,21 @@ public:
   // or not an event is pending.  When the resulting event is non-null, the
   // caller is responsible for releasing the event object.  This method does
   // not alter the reference count of the resulting event.
-  bool GetEvent(bool aMayWait, nsIRunnable** aEvent);
+  bool GetEvent(bool aMayWait, nsIRunnable** aEvent, MutexAutoLock&);
 
   // This method returns true if there is a pending event.
-  bool HasPendingEvent()
+  bool HasPendingEvent(MutexAutoLock& aProofOfLock)
   {
-    return GetEvent(false, nullptr);
+    return GetEvent(false, nullptr, aProofOfLock);
   }
 
   // This method returns the next pending event or null.
-  bool GetPendingEvent(nsIRunnable** aRunnable)
+  bool GetPendingEvent(nsIRunnable** aRunnable, MutexAutoLock& aProofOfLock)
   {
-    return GetEvent(false, aRunnable);
+    return GetEvent(false, aRunnable, aProofOfLock);
   }
 
-  size_t Count();
+  size_t Count(MutexAutoLock&);
 };
 
 #endif  // nsEventQueue_h__

xpcom/threads/nsThread.h

@@ -125,23 +125,23 @@ protected:
     bool GetEvent(bool aMayWait, nsIRunnable** aEvent,
                   mozilla::MutexAutoLock& aProofOfLock)
     {
-      return mQueue.GetEvent(aMayWait, aEvent);
+      return mQueue.GetEvent(aMayWait, aEvent, aProofOfLock);
     }
 
     void PutEvent(nsIRunnable* aEvent, mozilla::MutexAutoLock& aProofOfLock)
     {
-      mQueue.PutEvent(aEvent);
+      mQueue.PutEvent(aEvent, aProofOfLock);
     }
 
     void PutEvent(already_AddRefed<nsIRunnable>&& aEvent,
                   mozilla::MutexAutoLock& aProofOfLock)
     {
-      mQueue.PutEvent(mozilla::Move(aEvent));
+      mQueue.PutEvent(mozilla::Move(aEvent), aProofOfLock);
     }
 
     bool HasPendingEvent(mozilla::MutexAutoLock& aProofOfLock)
     {
-      return mQueue.HasPendingEvent();
+      return mQueue.HasPendingEvent(aProofOfLock);
     }
 
     nsChainedEventQueue* mNext;

xpcom/threads/nsThreadPool.cpp

@@ -49,6 +49,7 @@ NS_IMPL_CI_INTERFACE_GETTER(nsThreadPool, nsIThreadPool, nsIEventTarget)
 
 nsThreadPool::nsThreadPool()
   : mMonitor("[nsThreadPool.mMonitor]")
+  , mMutex("[nsThreadPool.mMutex]")
   , mThreadLimit(DEFAULT_THREAD_LIMIT)
   , mIdleThreadLimit(DEFAULT_IDLE_THREAD_LIMIT)
   , mIdleThreadTimeout(DEFAULT_IDLE_THREAD_TIMEOUT)
@@ -82,6 +83,7 @@ nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>&& aEvent)
   uint32_t stackSize = 0;
   {
     MonitorAutoLock mon(mMonitor);
+    MutexAutoLock lock(mMutex);
 
     if (NS_WARN_IF(mShutdown)) {
       return NS_ERROR_NOT_AVAILABLE;
@@ -94,11 +96,11 @@ nsThreadPool::PutEvent(already_AddRefed<nsIRunnable>&& aEvent)
     if (mThreads.Count() < (int32_t)mThreadLimit &&
         // Spawn a new thread if we don't have enough idle threads to serve
         // pending events immediately.
-        mEvents.Count(mon) >= mIdleCount) {
+        mEvents.Count(mon, lock) >= mIdleCount) {
       spawnThread = true;
     }
 
-    mEvents.PutEvent(Move(aEvent), mon);
+    mEvents.PutEvent(Move(aEvent), mon, lock);
     stackSize = mStackSize;
   }
 
@@ -179,7 +181,9 @@ nsThreadPool::Run()
     nsCOMPtr<nsIRunnable> event;
     {
       MonitorAutoLock mon(mMonitor);
-      if (!mEvents.GetPendingEvent(getter_AddRefs(event), mon)) {
+      MutexAutoLock lock(mMutex);
+
+      if (!mEvents.GetPendingEvent(getter_AddRefs(event), mon, lock)) {
         PRIntervalTime now     = PR_IntervalNow();
         PRIntervalTime timeout = PR_MillisecondsToInterval(mIdleThreadTimeout);
 

xpcom/threads/nsThreadPool.h

@@ -16,6 +16,7 @@
 #include "nsThreadUtils.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/AlreadyAddRefed.h"
+#include "mozilla/Mutex.h"
 #include "mozilla/Monitor.h"
 
 class nsThreadPool final
@@ -43,6 +44,7 @@ private:
 
   nsCOMArray<nsIThread> mThreads;
   mozilla::Monitor      mMonitor;
+  mozilla::Mutex        mMutex;
   nsEventQueueBase<mozilla::Monitor> mEvents;
   uint32_t              mThreadLimit;
   uint32_t              mIdleThreadLimit;