Bug 897033 - Fix nsCookieService's statement finalization. r=ehsan
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Tue, 23 Jul 2013 16:41:23 -0400
changeset 139708 421a27eb4647062bc40dca3507e087126940ed66
parent 139707 6745e2006d56999a5c3677fdbe50c990d93dac81
child 139709 67f1176844413d978d094d85679288901235bb11
push id25002
push useremorley@mozilla.com
push dateWed, 24 Jul 2013 12:36:15 +0000
treeherdermozilla-central@b3fcd828cadc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs897033
milestone25.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 897033 - Fix nsCookieService's statement finalization. r=ehsan
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -740,32 +740,33 @@ nsCookieService::InitDBStates()
 
   // Attempt to open and read the database. If TryInitDB() returns RESULT_RETRY,
   // do so.
   OpenDBResult result = TryInitDB(false);
   if (result == RESULT_RETRY) {
     // Database may be corrupt. Synchronously close the connection, clean up the
     // default DBState, and try again.
     COOKIE_LOGSTRING(PR_LOG_WARNING, ("InitDBStates(): retrying TryInitDB()"));
-
-    CloseDefaultDBConnection();
+    CleanupCachedStatements();
+    CleanupDefaultDBConnection();
     result = TryInitDB(true);
     if (result == RESULT_RETRY) {
       // We're done. Change the code to failure so we clean up below.
       result = RESULT_FAILURE;
     }
   }
 
   if (result == RESULT_FAILURE) {
     COOKIE_LOGSTRING(PR_LOG_WARNING,
       ("InitDBStates(): TryInitDB() failed, closing connection"));
 
     // Connection failure is unrecoverable. Clean up our connection. We can run
     // fine without persistent storage -- e.g. if there's no profile.
-    CloseDefaultDBConnection();
+    CleanupCachedStatements();
+    CleanupDefaultDBConnection();
   }
 }
 
 /* Attempt to open and read the database. If 'aRecreateDB' is true, try to
  * move the existing database file out of the way and create a new one.
  *
  * @returns RESULT_OK if opening or creating the database succeeded;
  *          RESULT_RETRY if the database cannot be opened, is corrupt, or some
@@ -1238,42 +1239,55 @@ nsCookieService::CloseDBStates()
   // Null out our private and pointer DBStates regardless.
   mPrivateDBState = NULL;
   mDBState = NULL;
 
   // If we don't have a default DBState, we're done.
   if (!mDefaultDBState)
     return;
 
+  // Cleanup cached statements before we can close anything.
+  CleanupCachedStatements();
+
   if (mDefaultDBState->dbConn) {
     // Cancel any pending read. No further results will be received by our
     // read listener.
     if (mDefaultDBState->pendingRead) {
       CancelAsyncRead(true);
     }
 
     // Asynchronously close the connection. We will null it below.
     mDefaultDBState->dbConn->AsyncClose(mDefaultDBState->closeListener);
   }
 
-  CloseDefaultDBConnection();
+  CleanupDefaultDBConnection();
 
   mDefaultDBState = NULL;
 }
 
-// Close the default connection by nulling out statements, listeners, and the
-// connection itself. This will not cancel a pending read or asynchronously
-// close the connection -- these must be done beforehand if necessary.
+// Null out the statements.
+// This must be done before closing the connection.
 void
-nsCookieService::CloseDefaultDBConnection()
+nsCookieService::CleanupCachedStatements()
 {
-  // Destroy our statements before we close the db.
-  mDefaultDBState->stmtInsert = NULL;
-  mDefaultDBState->stmtDelete = NULL;
-  mDefaultDBState->stmtUpdate = NULL;
+  mDefaultDBState->stmtInsert = nullptr;
+  mDefaultDBState->stmtDelete = nullptr;
+  mDefaultDBState->stmtUpdate = nullptr;
+}
+
+// Null out the listeners, and the database connection itself. This
+// will not null out the statements, cancel a pending read or
+// asynchronously close the connection -- these must be done
+// beforehand if necessary.
+void
+nsCookieService::CleanupDefaultDBConnection()
+{
+  MOZ_ASSERT(!mDefaultDBState->stmtInsert, "stmtInsert has been cleaned up");
+  MOZ_ASSERT(!mDefaultDBState->stmtDelete, "stmtDelete has been cleaned up");
+  MOZ_ASSERT(!mDefaultDBState->stmtUpdate, "stmtUpdate has been cleaned up");
 
   // Null out the database connections. If 'dbConn' has not been used for any
   // asynchronous operations yet, this will synchronously close it; otherwise,
   // it's expected that the caller has performed an AsyncClose prior.
   mDefaultDBState->dbConn = NULL;
   mDefaultDBState->syncConn = NULL;
 
   // Manually null out our listeners. This is necessary because they hold a
@@ -1349,32 +1363,34 @@ nsCookieService::HandleCorruptDB(DBState
     // idea how consistent the database is. Note that we may have already
     // canceled the read but not emptied our readSet; do so now.
     mDefaultDBState->readSet.Clear();
     if (mDefaultDBState->pendingRead) {
       CancelAsyncRead(true);
       mDefaultDBState->syncConn = nullptr;
     }
 
+    CleanupCachedStatements();
     mDefaultDBState->dbConn->AsyncClose(mDefaultDBState->closeListener);
-    CloseDefaultDBConnection();
+    CleanupDefaultDBConnection();
     break;
   }
   case DBState::CLOSING_FOR_REBUILD: {
     // We had an error while waiting for close completion. That's OK, just
     // ignore it -- we're rebuilding anyway.
     return;
   }
   case DBState::REBUILDING: {
     // We had an error while rebuilding the DB. Game over. Close the database
     // and let the close handler do nothing; then we'll move it out of the way.
+    CleanupCachedStatements();
     if (mDefaultDBState->dbConn) {
       mDefaultDBState->dbConn->AsyncClose(mDefaultDBState->closeListener);
     }
-    CloseDefaultDBConnection();
+    CleanupDefaultDBConnection();
     break;
   }
   }
 }
 
 static PLDHashOperator
 RebuildDBCallback(nsCookieEntry *aEntry,
                   void          *aArg)
@@ -1420,17 +1436,18 @@ nsCookieService::RebuildCorruptDB(DBStat
   // The database has been closed, and we're ready to rebuild. Open a
   // connection.
   OpenDBResult result = TryInitDB(true);
   if (result != RESULT_OK) {
     // We're done. Reset our DB connection and statements, and notify of
     // closure.
     COOKIE_LOGSTRING(PR_LOG_WARNING,
       ("RebuildCorruptDB(): TryInitDB() failed with result %u", result));
-    CloseDefaultDBConnection();
+    CleanupCachedStatements();
+    CleanupDefaultDBConnection();
     mDefaultDBState->corruptFlag = DBState::OK;
     mObserverService->NotifyObservers(nullptr, "cookie-db-closed", nullptr);
     return;
   }
 
   // Notify observers that we're beginning the rebuild.
   mObserverService->NotifyObservers(nullptr, "cookie-db-rebuilding", nullptr);
 
@@ -1450,17 +1467,17 @@ nsCookieService::RebuildCorruptDB(DBStat
     return;
   }
 
   // Execute the statement. If any errors crop up, we won't try again.
   DebugOnly<nsresult> rv = stmt->BindParameters(paramsArray);
   NS_ASSERT_SUCCESS(rv);
   nsCOMPtr<mozIStoragePendingStatement> handle;
   rv = stmt->ExecuteAsync(aDBState->insertListener, getter_AddRefs(handle));
-  NS_ASSERT_SUCCESS(rv);    
+  NS_ASSERT_SUCCESS(rv);
 }
 
 nsCookieService::~nsCookieService()
 {
   CloseDBStates();
 
   gCookieService = nullptr;
 }
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -248,17 +248,18 @@ class nsCookieService : public nsICookie
   static void AppClearDataObserverInit();
 
   protected:
     void                          PrefChanged(nsIPrefBranch *aPrefBranch);
     void                          InitDBStates();
     OpenDBResult                  TryInitDB(bool aDeleteExistingDB);
     nsresult                      CreateTable();
     void                          CloseDBStates();
-    void                          CloseDefaultDBConnection();
+    void                          CleanupCachedStatements();
+    void                          CleanupDefaultDBConnection();
     void                          HandleDBClosed(DBState* aDBState);
     void                          HandleCorruptDB(DBState* aDBState);
     void                          RebuildCorruptDB(DBState* aDBState);
     OpenDBResult                  Read();
     template<class T> nsCookie*   GetCookieFromRow(T &aRow);
     void                          AsyncReadComplete();
     void                          CancelAsyncRead(bool aPurgeReadSet);
     void                          EnsureReadDomain(const nsCookieKey &aKey);