Bug 831428 - Console listeners should not require a threadsafe addref/release method if they are only used from the main thread, r=jlebar
authorBenjamin Smedberg <benjamin@smedbergs.us>
Tue, 29 Jan 2013 11:02:56 -0500
changeset 130107 507d85ab580785783b54d88fc6e2bb7a8c13dc28
parent 130106 009b00bcce30d2bed015aa8af34ecae378fcb34c
child 130108 783ea16377776e5e538c8ad2492311a7a34d5bcb
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlebar
bugs831428
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 831428 - Console listeners should not require a threadsafe addref/release method if they are only used from the main thread, r=jlebar
xpcom/base/nsConsoleService.cpp
xpcom/base/nsConsoleService.h
--- a/xpcom/base/nsConsoleService.cpp
+++ b/xpcom/base/nsConsoleService.cpp
@@ -96,52 +96,54 @@ namespace {
 class LogMessageRunnable : public nsRunnable
 {
 public:
     LogMessageRunnable(nsIConsoleMessage* message, nsConsoleService* service)
         : mMessage(message)
         , mService(service)
     { }
 
-    void AddListener(nsIConsoleListener* listener) {
-        mListeners.AppendObject(listener);
-    }
-
     NS_DECL_NSIRUNNABLE
 
 private:
     nsCOMPtr<nsIConsoleMessage> mMessage;
     nsRefPtr<nsConsoleService> mService;
-    nsCOMArray<nsIConsoleListener> mListeners;
 };
 
+typedef nsCOMArray<nsIConsoleListener> ListenerArrayType;
+
+PLDHashOperator
+CollectCurrentListeners(nsISupports* aKey, nsIConsoleListener* aValue,
+                        void* closure)
+{
+    ListenerArrayType* listeners = static_cast<ListenerArrayType*>(closure);
+    listeners->AppendObject(aValue);
+    return PL_DHASH_NEXT;
+}
+
 NS_IMETHODIMP
 LogMessageRunnable::Run()
 {
     MOZ_ASSERT(NS_IsMainThread());
 
+    // Snapshot of listeners so that we don't reenter this hash during
+    // enumeration.
+    nsCOMArray<nsIConsoleListener> listeners;
+    mService->EnumerateListeners(CollectCurrentListeners, &listeners);
+
     mService->SetIsDelivering();
 
-    for (int32_t i = 0; i < mListeners.Count(); ++i)
-        mListeners[i]->Observe(mMessage);
+    for (int32_t i = 0; i < listeners.Count(); ++i)
+        listeners[i]->Observe(mMessage);
 
     mService->SetDoneDelivering();
 
     return NS_OK;
 }
 
-PLDHashOperator
-CollectCurrentListeners(nsISupports* aKey, nsIConsoleListener* aValue,
-                        void* closure)
-{
-    LogMessageRunnable* r = static_cast<LogMessageRunnable*>(closure);
-    r->AddListener(aValue);
-    return PL_DHASH_NEXT;
-}
-
 } // anonymous namespace
 
 // nsIConsoleService methods
 NS_IMETHODIMP
 nsConsoleService::LogMessage(nsIConsoleMessage *message)
 {
     return LogMessageWithMode(message, OutputToLog);
 }
@@ -200,38 +202,38 @@ nsConsoleService::LogMessageWithMode(nsI
         retiredMessage = mMessages[mCurrent];
         
         mMessages[mCurrent++] = message;
         if (mCurrent == mBufferSize) {
             mCurrent = 0; // wrap around.
             mFull = true;
         }
 
-        /*
-         * Copy the listeners into the snapshot array - in case a listener
-         * is removed during an Observe(...) notification. If there are no
-         * listeners, don't bother to create the Runnable, since we don't
-         * need to run it and it will hold onto the memory for the message
-         * unnecessarily.
-         */
         if (mListeners.Count() > 0) {
             r = new LogMessageRunnable(message, this);
-            mListeners.EnumerateRead(CollectCurrentListeners, r);
         }
     }
 
     if (retiredMessage != nullptr)
         NS_RELEASE(retiredMessage);
 
     if (r)
         NS_DispatchToMainThread(r);
 
     return NS_OK;
 }
 
+void
+nsConsoleService::EnumerateListeners(ListenerHash::EnumReadFunction aFunction,
+                                     void* aClosure)
+{
+    MutexAutoLock lock(mLock);
+    mListeners.EnumerateRead(aFunction, aClosure);
+}
+
 NS_IMETHODIMP
 nsConsoleService::LogStringMessage(const PRUnichar *message)
 {
     if (!sLoggingEnabled) {
         return NS_OK;
     }
 
     nsRefPtr<nsConsoleMessage> msg(new nsConsoleMessage(message));
--- a/xpcom/base/nsConsoleService.h
+++ b/xpcom/base/nsConsoleService.h
@@ -45,16 +45,19 @@ public:
     // B2G to control whether the message is logged to the android log or not.
 
     enum OutputMode {
         SuppressLog,
         OutputToLog
     };
     virtual nsresult LogMessageWithMode(nsIConsoleMessage *message, OutputMode outputMode);
 
+    typedef nsInterfaceHashtable<nsISupportsHashKey, nsIConsoleListener> ListenerHash;
+    void EnumerateListeners(ListenerHash::EnumReadFunction aFunction, void* aClosure);
+
 private:
     ~nsConsoleService();
 
     // Circular buffer of saved messages
     nsIConsoleMessage **mMessages;
 
     // How big?
     uint32_t mBufferSize;
@@ -66,15 +69,15 @@ private:
     bool mFull;
 
     // Are we currently delivering a console message on the main thread? If
     // so, we suppress incoming messages on the main thread only, to avoid
     // infinite repitition.
     bool mDeliveringMessage;
 
     // Listeners to notify whenever a new message is logged.
-    nsInterfaceHashtable<nsISupportsHashKey, nsIConsoleListener> mListeners;
+    ListenerHash mListeners;
 
     // To serialize interesting methods.
     mozilla::Mutex mLock;
 };
 
 #endif /* __nsconsoleservice_h__ */