Bug 675221 addendum to part A - reimplement the recursion check for the console service so that a poorly-written console listener doesn't cause an infinite repeition, r=bent
authorBenjamin Smedberg <benjamin@smedbergs.us>
Wed, 11 Jan 2012 11:28:21 -0500
changeset 85406 86da3ca1e3aefcc089892d8e9cab9d0a70c7e091
parent 85405 ab435575ac338cb5941e8add00398320f187ea2e
child 85407 2d79876ee1421bb02c92f6dc925c8f4f2298d207
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs675221
milestone12.0a1
Bug 675221 addendum to part A - reimplement the recursion check for the console service so that a poorly-written console listener doesn't cause an infinite repeition, r=bent
xpcom/base/nsConsoleService.cpp
xpcom/base/nsConsoleService.h
--- a/xpcom/base/nsConsoleService.cpp
+++ b/xpcom/base/nsConsoleService.cpp
@@ -63,16 +63,17 @@ NS_IMPL_THREADSAFE_RELEASE(nsConsoleServ
 NS_IMPL_CLASSINFO(nsConsoleService, NULL, nsIClassInfo::THREADSAFE | nsIClassInfo::SINGLETON, NS_CONSOLESERVICE_CID)
 NS_IMPL_QUERY_INTERFACE1_CI(nsConsoleService, nsIConsoleService)
 NS_IMPL_CI_INTERFACE_GETTER1(nsConsoleService, nsIConsoleService)
 
 nsConsoleService::nsConsoleService()
     : mMessages(nsnull)
     , mCurrent(0)
     , mFull(false)
+    , mDeliveringMessage(false)
     , mLock("nsConsoleService.mLock")
 {
     // XXX grab this from a pref!
     // hm, but worry about circularity, bc we want to be able to report
     // prefs errs...
     mBufferSize = 250;
 }
 
@@ -104,37 +105,45 @@ nsConsoleService::Init()
     return NS_OK;
 }
 
 namespace {
 
 class LogMessageRunnable : public nsRunnable
 {
 public:
-    LogMessageRunnable(nsIConsoleMessage* message) :
-        mMessage(message)
+    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;
 };
 
 NS_IMETHODIMP
 LogMessageRunnable::Run()
 {
+    MOZ_ASSERT(NS_IsMainThread());
+
+    mService->SetIsDelivering();
+
     for (PRInt32 i = 0; i < mListeners.Count(); ++i)
         mListeners[i]->Observe(mMessage);
 
+    mService->SetDoneDelivering();
+
     return NS_OK;
 }
 
 PLDHashOperator
 CollectCurrentListeners(nsISupports* aKey, nsIConsoleListener* aValue,
                         void* closure)
 {
     LogMessageRunnable* r = static_cast<LogMessageRunnable*>(closure);
@@ -146,17 +155,22 @@ CollectCurrentListeners(nsISupports* aKe
 
 // nsIConsoleService methods
 NS_IMETHODIMP
 nsConsoleService::LogMessage(nsIConsoleMessage *message)
 {
     if (message == nsnull)
         return NS_ERROR_INVALID_ARG;
 
-    nsRefPtr<LogMessageRunnable> r = new LogMessageRunnable(message);
+    if (NS_IsMainThread() && mDeliveringMessage) {
+        NS_WARNING("Some console listener threw an error while inside itself. Discarding this message");
+        return NS_ERROR_FAILURE;
+    }
+
+    nsRefPtr<LogMessageRunnable> r = new LogMessageRunnable(message, this);
     nsIConsoleMessage *retiredMessage;
 
     NS_ADDREF(message); // early, in case it's same as replaced below.
 
     /*
      * Lock while updating buffer, and while taking snapshot of
      * listeners array.
      */
--- a/xpcom/base/nsConsoleService.h
+++ b/xpcom/base/nsConsoleService.h
@@ -55,31 +55,48 @@ class nsConsoleService MOZ_FINAL : publi
 {
 public:
     nsConsoleService();
     nsresult Init();
 
     NS_DECL_ISUPPORTS
     NS_DECL_NSICONSOLESERVICE
 
+    void SetIsDelivering() {
+        MOZ_ASSERT(NS_IsMainThread());
+        MOZ_ASSERT(!mDeliveringMessage);
+        mDeliveringMessage = true;
+    }
+
+    void SetDoneDelivering() {
+        MOZ_ASSERT(NS_IsMainThread());
+        MOZ_ASSERT(mDeliveringMessage);
+        mDeliveringMessage = false;
+    }
+
 private:
     ~nsConsoleService();
 
     // Circular buffer of saved messages
     nsIConsoleMessage **mMessages;
 
     // How big?
     PRUint32 mBufferSize;
 
     // Index of slot in mMessages that'll be filled by *next* log message
     PRUint32 mCurrent;
 
     // Is the buffer full? (Has mCurrent wrapped around at least once?)
     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;
 
     // To serialize interesting methods.
     mozilla::Mutex mLock;
 };
 
 #endif /* __nsconsoleservice_h__ */