Bug 473845 - Break a potential cycle with XPConnect and mozStroageService
This changeset removes a cycle that gets created between XPConnect and the
storage service. This accomplishes this by releasing the cached reference to
XPConnect during xpcom-shutdown.
This was the last remaining leak of the browser-chrome test harness, so this
also sets the leak threshold to zero for that test.
r=bent
--- a/storage/src/mozStorageService.cpp
+++ b/storage/src/mozStorageService.cpp
@@ -44,22 +44,23 @@
#include "nsCRT.h"
#include "plstr.h"
#include "prinit.h"
#include "nsAutoLock.h"
#include "nsAutoPtr.h"
#include "nsEmbedCID.h"
#include "mozStoragePrivateHelpers.h"
#include "nsIXPConnect.h"
+#include "nsIObserverService.h"
#include "sqlite3.h"
#include "nsIPromptService.h"
-NS_IMPL_THREADSAFE_ISUPPORTS1(mozStorageService, mozIStorageService)
+NS_IMPL_THREADSAFE_ISUPPORTS2(mozStorageService, mozIStorageService, nsIObserver)
mozStorageService *mozStorageService::gStorageService = nsnull;
mozStorageService *
mozStorageService::GetSingleton()
{
if (gStorageService) {
NS_ADDREF(gStorageService);
@@ -88,79 +89,95 @@ mozStorageService::GetSingleton()
NS_ADDREF(gStorageService);
if (NS_FAILED(gStorageService->Init()))
NS_RELEASE(gStorageService);
}
return gStorageService;
}
-nsIXPConnect *mozStorageService::sXPConnect = nsnull;
-
-nsIXPConnect *
+already_AddRefed<nsIXPConnect>
mozStorageService::XPConnect()
{
- NS_ASSERTION(gStorageService,
- "Can not get XPConnect without an instance of our service!");
+ NS_ASSERTION(gStorageService,
+ "Can not get XPConnect without an instance of our service!");
- if (!sXPConnect) {
- (void)CallGetService(nsIXPConnect::GetCID(), &sXPConnect);
- NS_ASSERTION(sXPConnect, "Could not get XPConnect!");
- }
- return sXPConnect;
+ // If we've been shutdown, sXPConnect will be null. To prevent leaks, we do
+ // not cache the service after this point.
+ nsCOMPtr<nsIXPConnect> xpc(sXPConnect);
+ if (!xpc)
+ xpc = do_GetService(nsIXPConnect::GetCID());
+ NS_ASSERTION(xpc, "Could not get XPConnect!");
+ return xpc.forget();
}
mozStorageService::~mozStorageService()
{
// Shutdown the sqlite3 API. Warn if shutdown did not turn out okay, but
// there is nothing actionable we can do in that case.
int rc = sqlite3_shutdown();
if (rc != SQLITE_OK)
NS_WARNING("sqlite3 did not shutdown cleanly.");
-
+
gStorageService = nsnull;
PR_DestroyLock(mLock);
+}
+void
+mozStorageService::Shutdown()
+{
NS_IF_RELEASE(sXPConnect);
- sXPConnect = nsnull;
}
+nsIXPConnect *mozStorageService::sXPConnect = nsnull;
+
nsresult
mozStorageService::Init()
{
mLock = PR_NewLock();
if (!mLock)
return NS_ERROR_OUT_OF_MEMORY;
-
+
// Disable memory allocation statistic collection, improving performance.
// This must be done prior to a call to sqlite3_initialize to have any
// effect.
int rc = sqlite3_config(SQLITE_CONFIG_MEMSTATUS, 0);
if (rc != SQLITE_OK)
return ConvertResultCode(rc);
-
+
// Explicitly initialize sqlite3. Although this is implicitly called by
// various sqlite3 functions (and the sqlite3_open calls in our case),
// the documentation suggests calling this directly. So we do.
rc = sqlite3_initialize();
if (rc != SQLITE_OK)
return ConvertResultCode(rc);
// This makes multiple connections to the same database share the same pager
// cache. We do not need to lock here with mLock because this function is
// only ever called from mozStorageService::GetSingleton, which will only
// call this function once, and will not return until this function returns.
// (It does not matter where this is called relative to sqlite3_initialize.)
rc = sqlite3_enable_shared_cache(1);
if (rc != SQLITE_OK)
return ConvertResultCode(rc);
- return NS_OK;
+ nsCOMPtr<nsIObserverService> os =
+ do_GetService("@mozilla.org/observer-service;1");
+ NS_ENSURE_TRUE(os, NS_ERROR_FAILURE);
+
+ nsresult rv = os->AddObserver(this, "xpcom-shutdown", PR_FALSE);
+ NS_ENSURE_SUCCESS(rv, rv);
+
+ // We cache XPConnect for our language helpers.
+ (void)CallGetService(nsIXPConnect::GetCID(), &sXPConnect);
}
+////////////////////////////////////////////////////////////////////////////////
+//// mozIStorageService
+
#ifndef NS_APP_STORAGE_50_FILE
#define NS_APP_STORAGE_50_FILE "UStor"
#endif
/* mozIStorageConnection openSpecialDatabase(in string aStorageKey); */
NS_IMETHODIMP
mozStorageService::OpenSpecialDatabase(const char *aStorageKey, mozIStorageConnection **_retval)
{
@@ -284,8 +301,18 @@ mozStorageService::BackupDatabaseFile(ns
rv = backupDB->Remove(PR_FALSE);
NS_ENSURE_SUCCESS(rv, rv);
backupDB.swap(*backup);
return aDBFile->CopyTo(parentDir, fileName);
}
+////////////////////////////////////////////////////////////////////////////////
+//// nsIObserver
+
+NS_IMETHODIMP
+mozStorageService::Observe(nsISupports *, const char *aTopic, const PRUnichar *)
+{
+ if (strcmp(aTopic, "xpcom-shutdown") == 0)
+ Shutdown();
+ return NS_OK;
+}
--- a/storage/src/mozStorageService.h
+++ b/storage/src/mozStorageService.h
@@ -40,52 +40,56 @@
* ***** END LICENSE BLOCK ***** */
#ifndef _MOZSTORAGESERVICE_H_
#define _MOZSTORAGESERVICE_H_
#include "nsCOMPtr.h"
#include "nsIFile.h"
#include "nsIObserver.h"
-#include "nsIObserverService.h"
#include "prlock.h"
#include "mozIStorageService.h"
class mozStorageConnection;
class nsIXPConnect;
class mozStorageService : public mozIStorageService
+ , public nsIObserver
{
friend class mozStorageConnection;
public:
// two-phase init, must call before using service
nsresult Init();
static mozStorageService *GetSingleton();
- // nsISupports
NS_DECL_ISUPPORTS
-
- // mozIStorageService
NS_DECL_MOZISTORAGESERVICE
+ NS_DECL_NSIOBSERVER
/**
- * Obtains a pointer to XPConnect. This is used by language helpers.
+ * Obtains an already AddRefed pointer to XPConnect. This is used by
+ * language helpers.
*/
- static nsIXPConnect *XPConnect();
+ static already_AddRefed<nsIXPConnect> XPConnect();
private:
virtual ~mozStorageService();
/**
* Used for locking around calls when initializing connections so that we
* can ensure that the state of sqlite3_enable_shared_cache is sane.
*/
PRLock *mLock;
+
+ /**
+ * Shuts down the storage service, freeing all of the acquired resources.
+ */
+ void Shutdown();
protected:
nsCOMPtr<nsIFile> mProfileStorageFile;
static mozStorageService *gStorageService;
static nsIXPConnect *sXPConnect;
};
--- a/storage/src/mozStorageStatementJSHelper.cpp
+++ b/storage/src/mozStorageStatementJSHelper.cpp
@@ -51,18 +51,19 @@
#include "mozStorageStatementParams.h"
#include "jsapi.h"
static
JSBool
stepFunc(JSContext *aCtx, PRUint32, jsval *_vp)
{
+ nsCOMPtr<nsIXPConnect> xpc(mozStorageService::XPConnect());
nsCOMPtr<nsIXPConnectWrappedNative> wrapper;
- nsresult rv = mozStorageService::XPConnect()->GetWrappedNativeOfJSObject(
+ nsresult rv = xpc->GetWrappedNativeOfJSObject(
aCtx, JS_THIS_OBJECT(aCtx, _vp), getter_AddRefs(wrapper)
);
if (NS_FAILED(rv)) {
JS_ReportError(aCtx, "mozIStorageStatement::step() could not obtain native statement");
return JS_FALSE;
}
mozStorageStatement *stmt =
@@ -104,17 +105,18 @@ mozStorageStatementJSHelper::getRow(mozS
if (state != mozIStorageStatement::MOZ_STORAGE_STATEMENT_EXECUTING)
return NS_ERROR_UNEXPECTED;
if (!aStatement->mStatementRowHolder) {
nsCOMPtr<mozIStorageStatementRow> row =
new mozStorageStatementRow(aStatement);
NS_ENSURE_TRUE(row, NS_ERROR_OUT_OF_MEMORY);
- rv = mozStorageService::XPConnect()->WrapNative(
+ nsCOMPtr<nsIXPConnect> xpc(mozStorageService::XPConnect());
+ rv = xpc->WrapNative(
aCtx,
::JS_GetGlobalForObject(aCtx, aScopeObj),
row,
NS_GET_IID(mozIStorageStatementRow),
getter_AddRefs(aStatement->mStatementRowHolder)
);
NS_ENSURE_SUCCESS(rv, rv);
}
@@ -139,17 +141,18 @@ mozStorageStatementJSHelper::getParams(m
if (state != mozIStorageStatement::MOZ_STORAGE_STATEMENT_READY)
return NS_ERROR_UNEXPECTED;
if (!aStatement->mStatementParamsHolder) {
nsCOMPtr<mozIStorageStatementParams> params =
new mozStorageStatementParams(aStatement);
NS_ENSURE_TRUE(params, NS_ERROR_OUT_OF_MEMORY);
- rv = mozStorageService::XPConnect()->WrapNative(
+ nsCOMPtr<nsIXPConnect> xpc(mozStorageService::XPConnect());
+ rv = xpc->WrapNative(
aCtx,
::JS_GetGlobalForObject(aCtx, aScopeObj),
params,
NS_GET_IID(mozIStorageStatementParams),
getter_AddRefs(aStatement->mStatementParamsHolder)
);
NS_ENSURE_SUCCESS(rv, rv);
}
--- a/testing/mochitest/runtests.py.in
+++ b/testing/mochitest/runtests.py.in
@@ -460,19 +460,18 @@ def maybeForceLeakThreshold(options):
threshold can be successfully forced for the particular Mochitest type
and platform in use.
"""
if options.leakThreshold == INFINITY:
if options.chrome:
# We don't leak running the --chrome tests.
options.leakThreshold = 0
elif options.browserChrome:
- # We still leak a nondeterministic amount running browser-chrome tests.
- # But we are close to 0 (bug), so start to prevent/detect regressions. (Bug 460548)
- options.leakThreshold = 76500
+ # We don't leak running the --browser-chrome tests.
+ options.leakThreshold = 0
elif options.a11y:
# We don't leak running the --a11y tests.
options.leakThreshold = 0
else:
# Normal Mochitests: no leaks.
options.leakThreshold = 0
def makeTestConfig(options):