Bug 675221 part A: replace XPCOM proxies with runanble for code in XPCOM itself, r=bz
--- a/xpcom/base/nsConsoleService.cpp
+++ b/xpcom/base/nsConsoleService.cpp
@@ -40,17 +40,16 @@
* Maintains a circular buffer of recent messages, and notifies
* listeners when new messages are logged.
*/
/* Threadsafe. */
#include "nsMemory.h"
#include "nsIServiceManager.h"
-#include "nsIProxyObjectManager.h"
#include "nsCOMArray.h"
#include "nsThreadUtils.h"
#include "nsConsoleService.h"
#include "nsConsoleMessage.h"
#include "nsIClassInfoImpl.h"
#if defined(ANDROID)
@@ -61,78 +60,103 @@ using namespace mozilla;
NS_IMPL_THREADSAFE_ADDREF(nsConsoleService)
NS_IMPL_THREADSAFE_RELEASE(nsConsoleService)
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), mListening(false), mLock("nsConsoleService.mLock")
+ : mMessages(nsnull)
+ , mCurrent(0)
+ , mFull(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;
}
nsConsoleService::~nsConsoleService()
{
PRUint32 i = 0;
while (i < mBufferSize && mMessages[i] != nsnull) {
NS_RELEASE(mMessages[i]);
i++;
}
-#ifdef DEBUG_mccabe
- if (mListeners.Count() != 0) {
- fprintf(stderr,
- "WARNING - %d console error listeners still registered!\n"
- "More calls to nsIConsoleService::UnregisterListener needed.\n",
- mListeners.Count());
- }
-
-#endif
-
if (mMessages)
nsMemory::Free(mMessages);
}
nsresult
nsConsoleService::Init()
{
mMessages = (nsIConsoleMessage **)
nsMemory::Alloc(mBufferSize * sizeof(nsIConsoleMessage *));
if (!mMessages)
return NS_ERROR_OUT_OF_MEMORY;
// Array elements should be 0 initially for circular buffer algorithm.
memset(mMessages, 0, mBufferSize * sizeof(nsIConsoleMessage *));
+ mListeners.Init();
+
return NS_OK;
}
-static bool snapshot_enum_func(nsHashKey *key, void *data, void* closure)
+namespace {
+
+class LogMessageRunnable : public nsRunnable
{
- nsCOMArray<nsIConsoleListener> *array =
- reinterpret_cast<nsCOMArray<nsIConsoleListener> *>(closure);
+public:
+ LogMessageRunnable(nsIConsoleMessage* message) :
+ mMessage(message)
+ { }
+
+ void AddListener(nsIConsoleListener* listener) {
+ mListeners.AppendObject(listener);
+ }
+
+ NS_DECL_NSIRUNNABLE
+
+private:
+ nsCOMPtr<nsIConsoleMessage> mMessage;
+ nsCOMArray<nsIConsoleListener> mListeners;
+};
- // Copy each element into the temporary nsCOMArray...
- array->AppendObject((nsIConsoleListener*)data);
- return true;
+NS_IMETHODIMP
+LogMessageRunnable::Run()
+{
+ for (PRInt32 i = 0; i < mListeners.Count(); ++i)
+ mListeners[i]->Observe(mMessage);
+
+ 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)
{
if (message == nsnull)
return NS_ERROR_INVALID_ARG;
- nsCOMArray<nsIConsoleListener> listenersSnapshot;
+ nsRefPtr<LogMessageRunnable> r = new LogMessageRunnable(message);
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.
*/
@@ -161,46 +185,22 @@ nsConsoleService::LogMessage(nsIConsoleM
mCurrent = 0; // wrap around.
mFull = true;
}
/*
* Copy the listeners into the snapshot array - in case a listener
* is removed during an Observe(...) notification...
*/
- mListeners.Enumerate(snapshot_enum_func, &listenersSnapshot);
+ mListeners.EnumerateRead(CollectCurrentListeners, r);
}
if (retiredMessage != nsnull)
NS_RELEASE(retiredMessage);
- /*
- * Iterate through any registered listeners and tell them about
- * the message. We use the mListening flag to guard against
- * recursive message logs. This could sometimes result in
- * listeners being skipped because of activity on other threads,
- * when we only care about the recursive case.
- */
- nsCOMPtr<nsIConsoleListener> listener;
- PRInt32 snapshotCount = listenersSnapshot.Count();
-
- {
- MutexAutoLock lock(mLock);
- if (mListening)
- return NS_OK;
- mListening = true;
- }
-
- for (PRInt32 i = 0; i < snapshotCount; i++) {
- listenersSnapshot[i]->Observe(message);
- }
-
- {
- MutexAutoLock lock(mLock);
- mListening = false;
- }
+ NS_DispatchToMainThread(r);
return NS_OK;
}
NS_IMETHODIMP
nsConsoleService::LogStringMessage(const PRUnichar *message)
{
nsConsoleMessage *msg = new nsConsoleMessage(message);
@@ -260,73 +260,54 @@ nsConsoleService::GetMessageArray(nsICon
}
*count = resultSize;
*messages = messageArray;
return NS_OK;
}
NS_IMETHODIMP
-nsConsoleService::RegisterListener(nsIConsoleListener *listener) {
- nsresult rv;
-
- /*
- * Store a threadsafe proxy to the listener rather than the
- * listener itself; we want the console service to be callable
- * from any thread, but listeners can be implemented in
- * thread-specific ways, and we always want to call them on their
- * originating thread. JavaScript is the motivating example.
- */
- nsCOMPtr<nsIConsoleListener> proxiedListener;
-
- rv = GetProxyForListener(listener, getter_AddRefs(proxiedListener));
- if (NS_FAILED(rv))
- return rv;
+nsConsoleService::RegisterListener(nsIConsoleListener *listener)
+{
+ if (!NS_IsMainThread()) {
+ NS_ERROR("nsConsoleService::RegisterListener is main thread only.");
+ return NS_ERROR_NOT_SAME_THREAD;
+ }
- {
- MutexAutoLock lock(mLock);
- nsISupportsKey key(listener);
+ nsCOMPtr<nsISupports> canonical = do_QueryInterface(listener);
- /*
- * Put the proxy event listener into a hashtable using the *real*
- * listener as the key.
- *
- * This is necessary because proxy objects do *not* maintain
- * nsISupports identity. Therefore, since GetProxyForListener(...)
- * can return different proxies for the same object (see bug #85831)
- * we need to use the real object as the unique key...
- */
- mListeners.Put(&key, proxiedListener);
+ MutexAutoLock lock(mLock);
+ if (mListeners.GetWeak(canonical)) {
+ // Reregistering a listener isn't good
+ return NS_ERROR_FAILURE;
}
+ mListeners.Put(canonical, listener);
return NS_OK;
}
NS_IMETHODIMP
-nsConsoleService::UnregisterListener(nsIConsoleListener *listener) {
+nsConsoleService::UnregisterListener(nsIConsoleListener *listener)
+{
+ if (!NS_IsMainThread()) {
+ NS_ERROR("nsConsoleService::UnregisterListener is main thread only.");
+ return NS_ERROR_NOT_SAME_THREAD;
+ }
+
+ nsCOMPtr<nsISupports> canonical = do_QueryInterface(listener);
+
MutexAutoLock lock(mLock);
- nsISupportsKey key(listener);
- mListeners.Remove(&key);
+ if (!mListeners.GetWeak(canonical)) {
+ // Unregistering a listener that was never registered?
+ return NS_ERROR_FAILURE;
+ }
+ mListeners.Remove(canonical);
return NS_OK;
}
-nsresult
-nsConsoleService::GetProxyForListener(nsIConsoleListener* aListener,
- nsIConsoleListener** aProxy)
-{
- /*
- * Would it be better to catch that case and leave the listener unproxied?
- */
- return NS_GetProxyForObject(NS_PROXY_TO_CURRENT_THREAD,
- NS_GET_IID(nsIConsoleListener),
- aListener,
- NS_PROXY_ASYNC | NS_PROXY_ALWAYS,
- (void**) aProxy);
-}
-
NS_IMETHODIMP
nsConsoleService::Reset()
{
/*
* Make sure nobody trips into the buffer while it's being reset
*/
MutexAutoLock lock(mLock);
--- a/xpcom/base/nsConsoleService.h
+++ b/xpcom/base/nsConsoleService.h
@@ -41,52 +41,45 @@
#ifndef __nsconsoleservice_h__
#define __nsconsoleservice_h__
#include "mozilla/Attributes.h"
#include "mozilla/Mutex.h"
#include "nsCOMPtr.h"
-#include "nsHashtable.h"
+#include "nsInterfaceHashtable.h"
+#include "nsHashKeys.h"
#include "nsIConsoleService.h"
class nsConsoleService MOZ_FINAL : public nsIConsoleService
{
public:
nsConsoleService();
nsresult Init();
NS_DECL_ISUPPORTS
NS_DECL_NSICONSOLESERVICE
private:
~nsConsoleService();
- // build (or find) a proxy for the listener
- nsresult GetProxyForListener(nsIConsoleListener* aListener,
- nsIConsoleListener** aProxy);
-
// 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;
// Listeners to notify whenever a new message is logged.
- nsSupportsHashtable mListeners;
-
- // Current listener being notified of a logged error - to prevent
- // stack overflows.
- bool mListening;
+ nsInterfaceHashtable<nsISupportsHashKey, nsIConsoleListener> mListeners;
// To serialize interesting methods.
mozilla::Mutex mLock;
};
#endif /* __nsconsoleservice_h__ */
--- a/xpcom/components/nsCategoryManager.cpp
+++ b/xpcom/components/nsCategoryManager.cpp
@@ -54,17 +54,16 @@
#include "nsComponentManagerUtils.h"
#include "nsServiceManagerUtils.h"
#include "nsIObserver.h"
#include "nsIObserverService.h"
#include "nsReadableUtils.h"
#include "nsCRT.h"
#include "nsQuickSort.h"
#include "nsEnumeratorUtils.h"
-#include "nsIProxyObjectManager.h"
#include "nsThreadUtils.h"
#include "mozilla/Services.h"
#include "ManifestParser.h"
#include "mozilla/FunctionTimer.h"
using namespace mozilla;
class nsIComponentLoaderManager;
@@ -492,53 +491,81 @@ inline CategoryNode*
nsCategoryManager::get_category(const char* aName) {
CategoryNode* node;
if (!mTable.Get(aName, &node)) {
return nsnull;
}
return node;
}
+namespace {
+
+class CategoryNotificationRunnable : public nsRunnable
+{
+public:
+ CategoryNotificationRunnable(nsISupports* aSubject,
+ const char* aTopic,
+ const char* aData)
+ : mSubject(aSubject)
+ , mTopic(aTopic)
+ , mData(aData)
+ { }
+
+ NS_DECL_NSIRUNNABLE
+
+private:
+ nsCOMPtr<nsISupports> mSubject;
+ const char* mTopic;
+ NS_ConvertUTF8toUTF16 mData;
+};
+
+NS_IMETHODIMP
+CategoryNotificationRunnable::Run()
+{
+ nsCOMPtr<nsIObserverService> observerService =
+ mozilla::services::GetObserverService();
+ if (observerService)
+ observerService->NotifyObservers(mSubject, mTopic, mData.get());
+
+ return NS_OK;
+}
+
+} // anonymous namespace
+
+
void
nsCategoryManager::NotifyObservers( const char *aTopic,
const char *aCategoryName,
const char *aEntryName )
{
if (mSuppressNotifications)
return;
- nsCOMPtr<nsIObserverService> observerService =
- mozilla::services::GetObserverService();
- if (!observerService)
- return;
-
- nsCOMPtr<nsIObserverService> obsProxy;
- NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
- NS_GET_IID(nsIObserverService),
- observerService,
- NS_PROXY_ASYNC,
- getter_AddRefs(obsProxy));
- if (!obsProxy)
- return;
+ nsRefPtr<CategoryNotificationRunnable> r;
if (aEntryName) {
nsCOMPtr<nsISupportsCString> entry
(do_CreateInstance (NS_SUPPORTS_CSTRING_CONTRACTID));
if (!entry)
return;
nsresult rv = entry->SetData(nsDependentCString(aEntryName));
if (NS_FAILED(rv))
return;
- obsProxy->NotifyObservers(entry, aTopic,
- NS_ConvertUTF8toUTF16(aCategoryName).get());
+ r = new CategoryNotificationRunnable(entry, aTopic, aCategoryName);
} else {
- obsProxy->NotifyObservers(this, aTopic,
- NS_ConvertUTF8toUTF16(aCategoryName).get());
+ r = new CategoryNotificationRunnable(this, aTopic, aCategoryName);
+ }
+
+ if (NS_IsMainThread()) {
+ r->Run();
+ }
+ else {
+ NS_DispatchToMainThread(r);
}
}
NS_IMETHODIMP
nsCategoryManager::GetCategoryEntry( const char *aCategoryName,
const char *aEntryName,
char **_retval )
{
--- a/xpcom/components/nsCategoryManager.h
+++ b/xpcom/components/nsCategoryManager.h
@@ -152,17 +152,17 @@ public:
private:
static nsCategoryManager* gCategoryManager;
nsCategoryManager();
~nsCategoryManager();
CategoryNode* get_category(const char* aName);
void NotifyObservers(const char* aTopic,
- const char* aCategoryName,
+ const char* aCategoryName, // must be a static string
const char* aEntryName);
PLArenaPool mArena;
nsClassHashtable<nsDepCharHashKey, CategoryNode> mTable;
mozilla::Mutex mLock;
bool mSuppressNotifications;
};
--- a/xpcom/components/nsNativeComponentLoader.cpp
+++ b/xpcom/components/nsNativeComponentLoader.cpp
@@ -59,17 +59,16 @@
#include "nsComponentManager.h"
#include "ManifestParser.h" // for LogMessage
#include "nsCRTGlue.h"
#include "nsThreadUtils.h"
#include "nsTraceRefcntImpl.h"
#include "nsILocalFile.h"
-#include "nsIProxyObjectManager.h"
#ifdef XP_WIN
#include <windows.h>
#endif
#ifdef XP_MACOSX
#include <signal.h>
#endif
--- a/xpcom/tests/TestThreadPoolListener.cpp
+++ b/xpcom/tests/TestThreadPoolListener.cpp
@@ -32,17 +32,16 @@
* and other provisions required by the GPL or the LGPL. If you do not delete
* the provisions above, a recipient may use your version of this file under
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
#include "TestHarness.h"
-#include "nsIProxyObjectManager.h"
#include "nsIThread.h"
#include "nsIThreadPool.h"
#include "nsThreadUtils.h"
#include "nsXPCOMCIDInternal.h"
#include "pratom.h"
#include "prinrval.h"
#include "prmon.h"
@@ -173,23 +172,16 @@ int main(int argc, char** argv)
nsIThread* shutDownThreadList[NUMBER_OF_THREADS] = { nsnull };
gShutDownThreadList = shutDownThreadList;
AutoCreateAndDestroyReentrantMonitor newMon(&gReentrantMonitor);
NS_ENSURE_TRUE(gReentrantMonitor, 1);
nsresult rv;
- // Grab the proxy service before messing with the thread pool. This is a
- // workaround for bug 449822 where the thread pool shutdown can create two
- // instances of the proxy service and hang.
- nsCOMPtr<nsIProxyObjectManager> proxyObjMgr =
- do_GetService(NS_XPCOMPROXY_CONTRACTID, &rv);
- NS_ENSURE_SUCCESS(rv, rv);
-
nsCOMPtr<nsIThreadPool> pool =
do_CreateInstance(NS_THREADPOOL_CONTRACTID, &rv);
NS_ENSURE_SUCCESS(rv, 1);
rv = pool->SetThreadLimit(NUMBER_OF_THREADS);
NS_ENSURE_SUCCESS(rv, 1);
rv = pool->SetIdleThreadLimit(NUMBER_OF_THREADS);
--- a/xpcom/threads/TimerThread.cpp
+++ b/xpcom/threads/TimerThread.cpp
@@ -41,17 +41,16 @@
#include "nsTimerImpl.h"
#include "TimerThread.h"
#include "nsThreadUtils.h"
#include "pratom.h"
#include "nsIObserverService.h"
#include "nsIServiceManager.h"
-#include "nsIProxyObjectManager.h"
#include "mozilla/Services.h"
#include <math.h>
using namespace mozilla;
NS_IMPL_THREADSAFE_ISUPPORTS2(TimerThread, nsIRunnable, nsIObserver)
@@ -75,16 +74,45 @@ TimerThread::~TimerThread()
}
nsresult
TimerThread::InitLocks()
{
return NS_OK;
}
+namespace {
+
+class TimerObserverRunnable : public nsRunnable
+{
+public:
+ TimerObserverRunnable(nsIObserver* observer)
+ : mObserver(observer)
+ { }
+
+ NS_DECL_NSIRUNNABLE
+
+private:
+ nsCOMPtr<nsIObserver> mObserver;
+};
+
+NS_IMETHODIMP
+TimerObserverRunnable::Run()
+{
+ nsCOMPtr<nsIObserverService> observerService =
+ mozilla::services::GetObserverService();
+ if (observerService) {
+ observerService->AddObserver(mObserver, "sleep_notification", PR_FALSE);
+ observerService->AddObserver(mObserver, "wake_notification", PR_FALSE);
+ }
+ return NS_OK;
+}
+
+} // anonymous namespace
+
nsresult TimerThread::Init()
{
PR_LOG(gTimerLog, PR_LOG_DEBUG, ("TimerThread::Init [%d]\n", mInitialized));
if (mInitialized) {
if (!mThread)
return NS_ERROR_FAILURE;
@@ -93,31 +121,22 @@ nsresult TimerThread::Init()
if (PR_ATOMIC_SET(&mInitInProgress, 1) == 0) {
// We hold on to mThread to keep the thread alive.
nsresult rv = NS_NewThread(getter_AddRefs(mThread), this);
if (NS_FAILED(rv)) {
mThread = nsnull;
}
else {
- nsCOMPtr<nsIObserverService> observerService =
- mozilla::services::GetObserverService();
- // We must not use the observer service from a background thread!
- if (observerService && !NS_IsMainThread()) {
- nsCOMPtr<nsIObserverService> result = nsnull;
- NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD,
- NS_GET_IID(nsIObserverService),
- observerService, NS_PROXY_ASYNC,
- getter_AddRefs(result));
- observerService.swap(result);
+ nsRefPtr<TimerObserverRunnable> r = new TimerObserverRunnable(this);
+ if (NS_IsMainThread()) {
+ r->Run();
}
- // We'll be released at xpcom shutdown
- if (observerService) {
- observerService->AddObserver(this, "sleep_notification", false);
- observerService->AddObserver(this, "wake_notification", false);
+ else {
+ NS_DispatchToMainThread(r);
}
}
{
MonitorAutoLock lock(mMonitor);
mInitialized = true;
mMonitor.NotifyAll();
}
--- a/xpcom/threads/nsThreadPool.cpp
+++ b/xpcom/threads/nsThreadPool.cpp
@@ -31,17 +31,16 @@
* use your version of this file under the terms of the MPL, indicate your
* decision by deleting the provisions above and replace them with the notice
* and other provisions required by the GPL or the LGPL. If you do not delete
* the provisions above, a recipient may use your version of this file under
* the terms of any one of the MPL, the GPL or the LGPL.
*
* ***** END LICENSE BLOCK ***** */
-#include "nsIProxyObjectManager.h"
#include "nsIClassInfoImpl.h"
#include "nsThreadPool.h"
#include "nsThreadManager.h"
#include "nsThread.h"
#include "nsMemory.h"
#include "nsAutoPtr.h"
#include "prinrval.h"
#include "prlog.h"
@@ -139,24 +138,18 @@ nsThreadPool::ShutdownThread(nsIThread *
{
LOG(("THRD-P(%p) shutdown async [%p]\n", this, thread));
// This method is responsible for calling Shutdown on |thread|. This must be
// done from some other thread, so we use the main thread of the application.
NS_ASSERTION(!NS_IsMainThread(), "wrong thread");
- nsCOMPtr<nsIThread> doomed;
- NS_GetProxyForObject(NS_PROXY_TO_MAIN_THREAD, NS_GET_IID(nsIThread), thread,
- NS_PROXY_ASYNC, getter_AddRefs(doomed));
- if (doomed) {
- doomed->Shutdown();
- } else {
- NS_WARNING("failed to construct proxy to main thread");
- }
+ nsRefPtr<nsIRunnable> r = NS_NewRunnableMethod(thread, &nsIThread::Shutdown);
+ NS_DispatchToMainThread(r);
}
NS_IMETHODIMP
nsThreadPool::Run()
{
LOG(("THRD-P(%p) enter\n", this));
nsCOMPtr<nsIThread> current;