Bug 496019 - mozilla::storage::Connection::Close can spin a nested event loop
authorShawn Wilsher <sdwilsh@shawnwilsher.com>
Mon, 09 Nov 2009 09:58:34 -0800
changeset 34690 15a2aadd04ddbdaf9c433f253187492c0ed246ae
parent 34689 3776e2ddf1a38f3d84558c7cdfa113f168fd5f84
child 34691 551abc8ab1aa0e2bae5f05385b8e7da5c45bf807
push idunknown
push userunknown
push dateunknown
bugs496019
milestone1.9.3a1pre
Bug 496019 - mozilla::storage::Connection::Close can spin a nested event loop Creates a new close method that must be used when using asynchronous statements, and disallow Close from being called in that case. r=asuth sr=vlad
storage/public/Makefile.in
storage/public/mozIStorageCompletionCallback.idl
storage/public/mozIStorageConnection.idl
storage/src/mozStorageAsyncStatementExecution.cpp
storage/src/mozStorageConnection.cpp
storage/src/mozStorageConnection.h
storage/test/unit/test_connection_executeAsync.js
storage/test/unit/test_storage_connection.js
--- a/storage/public/Makefile.in
+++ b/storage/public/Makefile.in
@@ -59,16 +59,17 @@ XPIDLSRCS = \
 	mozIStorageValueArray.idl \
 	mozIStorageResultSet.idl \
 	mozIStorageRow.idl \
   mozIStorageError.idl \
   mozIStorageStatementCallback.idl \
   mozIStoragePendingStatement.idl \
   mozIStorageBindingParamsArray.idl \
   mozIStorageBindingParams.idl \
+  mozIStorageCompletionCallback.idl \
 	$(NULL)
 
 EXPORTS_NAMESPACES = mozilla
 
 EXPORTS = \
 	mozStorageHelper.h \
 	mozStorage.h \
 	$(NULL)
new file mode 100644
--- /dev/null
+++ b/storage/public/mozIStorageCompletionCallback.idl
@@ -0,0 +1,48 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*-
+  vim: sw=2 ts=2 et lcs=trail\:.,tab\:>~ :
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Shawn Wilsher <me@shawnwilsher.com> (Original Author)
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * 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 "nsISupports.idl"
+
+[scriptable,function, uuid(0bfee0c4-2c24-400e-b18e-b5bb41a032c8)]
+interface mozIStorageCompletionCallback : nsISupports {
+  /**
+   * Indicates that the event this callback was passed in for has completed.
+   */
+  void complete();
+};
--- a/storage/public/mozIStorageConnection.idl
+++ b/storage/public/mozIStorageConnection.idl
@@ -37,43 +37,58 @@
  * 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 "nsISupports.idl"
 
 interface mozIStorageAggregateFunction;
+interface mozIStorageCompletionCallback;
 interface mozIStorageFunction;
 interface mozIStorageProgressHandler;
 interface mozIStorageStatement;
 interface mozIStorageStatementCallback;
 interface mozIStoragePendingStatement;
 interface nsIFile;
 
 /**
  * mozIStorageConnection represents a database connection attached to
  * a specific file or to the in-memory data storage.  It is the
  * primary interface for interacting with a database, including
  * creating prepared statements, executing SQL, and examining database
  * errors.
  *
  * @threadsafe
  */
-[scriptable, uuid(ac3c486c-69a1-4cbe-8f25-2ad20880eab3)]
+[scriptable, uuid(5a06b207-1977-47d4-b140-271cb851bb26)]
 interface mozIStorageConnection : nsISupports {
   /*
    * Initialization and status
    */
 
   /**
-   * Closes a database connection.  C++ callers should simply set the database
-   * variable to NULL.
+   * Closes a database connection.  Callers must finalize all statements created
+   * for this connection prior to calling this method.  It is illegal to use
+   * call this method if any asynchronous statements have been executed on this
+   * connection.
+   *
+   * @throws NS_ERROR_UNEXPECTED
+   *         If any statement has been executed asynchronously on this object.
    */
-   void close();
+  void close();
+
+  /**
+   * Asynchronously closes a database connection, allowing all pending
+   * asynchronous statements to complete first.
+   *
+   * @param aCallback [optional]
+   *        A callback that will be notified when the close is completed.
+   */
+  void asyncClose([optional] in mozIStorageCompletionCallback aCallback);
 
   /**
    * Indicates if the connection is open and ready to use.  This will be false
    * if the connection failed to open, or it has been closed.
    */
   readonly attribute boolean connectionReady;
 
   /**
--- a/storage/src/mozStorageAsyncStatementExecution.cpp
+++ b/storage/src/mozStorageAsyncStatementExecution.cpp
@@ -185,17 +185,17 @@ AsyncExecuteStatements::execute(Statemen
   NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
 
   // Dispatch it to the background
   nsCOMPtr<nsIEventTarget> target(aConnection->getAsyncExecutionTarget());
   NS_ENSURE_TRUE(target, NS_ERROR_NOT_AVAILABLE);
   nsresult rv = target->Dispatch(event, NS_DISPATCH_NORMAL);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Return it as the pending statement object
+  // Return it as the pending statement object and track it.
   NS_ADDREF(*_stmt = event);
   return NS_OK;
 }
 
 AsyncExecuteStatements::AsyncExecuteStatements(StatementDataArray &aStatements,
                                                Connection *aConnection,
                                                mozIStorageStatementCallback *aCallback)
 : mConnection(aConnection)
--- a/storage/src/mozStorageConnection.cpp
+++ b/storage/src/mozStorageConnection.cpp
@@ -49,16 +49,17 @@
 #include "nsAutoPtr.h"
 #include "nsIFile.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsThreadUtils.h"
 #include "nsAutoLock.h"
 
 #include "mozIStorageAggregateFunction.h"
+#include "mozIStorageCompletionCallback.h"
 #include "mozIStorageFunction.h"
 
 #include "mozStorageAsyncStatementExecution.h"
 #include "mozStorageSQLFunctions.h"
 #include "mozStorageConnection.h"
 #include "mozStorageService.h"
 #include "mozStorageStatement.h"
 #include "mozStorageArgValueArray.h"
@@ -233,55 +234,86 @@ aggregateFunctionFinalHelper(sqlite3_con
   if (variantToSQLiteT(aCtx, result) != SQLITE_OK) {
     NS_WARNING("User aggregate final function returned invalid data type!");
     ::sqlite3_result_error(aCtx,
                            "User aggregate final function returned invalid data type",
                            -1);
   }
 }
 
+} // anonymous namespace
+
+////////////////////////////////////////////////////////////////////////////////
+//// Local Classes
+
+namespace {
+
+class AsyncCloseConnection : public nsRunnable
+{
+public:
+  AsyncCloseConnection(Connection *aConnection,
+                       nsIEventTarget *aCallingThread,
+                       nsIRunnable *aCallbackEvent)
+  : mConnection(aConnection)
+  , mCallingThread(aCallingThread)
+  , mCallbackEvent(aCallbackEvent)
+  {
+  }
+
+  NS_METHOD Run()
+  {
+    (void)mConnection->internalClose();
+    if (mCallbackEvent)
+      (void)mCallingThread->Dispatch(mCallbackEvent, NS_DISPATCH_NORMAL);
+
+    return NS_OK;
+  }
+private:
+  nsCOMPtr<Connection> mConnection;
+  nsCOMPtr<nsIEventTarget> mCallingThread;
+  nsCOMPtr<nsIRunnable> mCallbackEvent;
+};
 
 } // anonymous namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Connection
 
 Connection::Connection(Service *aService)
 : sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex")
 , mDBConn(nsnull)
-, mAsyncExecutionMutex(nsAutoLock::NewLock("AsyncExecutionMutex"))
-, mAsyncExecutionThreadShuttingDown(PR_FALSE)
+, mAsyncExecutionMutex("Connection::mAsyncExecutionMutex")
+, mAsyncExecutionThreadShuttingDown(false)
 , mTransactionMutex(nsAutoLock::NewLock("TransactionMutex"))
 , mTransactionInProgress(PR_FALSE)
 , mFunctionsMutex(nsAutoLock::NewLock("FunctionsMutex"))
 , mProgressHandlerMutex(nsAutoLock::NewLock("ProgressHandlerMutex"))
 , mProgressHandler(nsnull)
 , mStorageService(aService)
 {
   mFunctions.Init();
 }
 
 Connection::~Connection()
 {
   (void)Close();
-  nsAutoLock::DestroyLock(mAsyncExecutionMutex);
   nsAutoLock::DestroyLock(mTransactionMutex);
   nsAutoLock::DestroyLock(mFunctionsMutex);
   nsAutoLock::DestroyLock(mProgressHandlerMutex);
 }
 
 NS_IMPL_THREADSAFE_ISUPPORTS1(
   Connection,
   mozIStorageConnection
 )
 
 already_AddRefed<nsIEventTarget>
 Connection::getAsyncExecutionTarget()
 {
-  nsAutoLock mutex(mAsyncExecutionMutex);
+  MutexAutoLock lockedScope(mAsyncExecutionMutex);
 
   // If we are shutting down the asynchronous thread, don't hand out any more
   // references to the thread.
   if (mAsyncExecutionThreadShuttingDown)
     return nsnull;
 
   if (!mAsyncExecutionThread) {
     nsresult rv = ::NS_NewThread(getter_AddRefs(mAsyncExecutionThread));
@@ -295,17 +327,16 @@ Connection::getAsyncExecutionTarget()
   NS_ADDREF(eventTarget = mAsyncExecutionThread);
   return eventTarget;
 }
 
 nsresult
 Connection::initialize(nsIFile *aDatabaseFile)
 {
   NS_ASSERTION (!mDBConn, "Initialize called on already opened database!");
-  NS_ENSURE_TRUE(mAsyncExecutionMutex, NS_ERROR_OUT_OF_MEMORY);
   NS_ENSURE_TRUE(mTransactionMutex, NS_ERROR_OUT_OF_MEMORY);
   NS_ENSURE_TRUE(mFunctionsMutex, NS_ERROR_OUT_OF_MEMORY);
   NS_ENSURE_TRUE(mProgressHandlerMutex, NS_ERROR_OUT_OF_MEMORY);
 
   int srv;
   nsresult rv;
 
   mDatabaseFile = aDatabaseFile;
@@ -465,72 +496,122 @@ Connection::progressHandler()
     PRBool result;
     nsresult rv = mProgressHandler->OnProgress(this, &result);
     if (NS_FAILED(rv)) return 0; // Don't break request
     return result ? 1 : 0;
   }
   return 0;
 }
 
-////////////////////////////////////////////////////////////////////////////////
-//// mozIStorageConnection
+nsresult
+Connection::setClosedState()
+{
+  // Flag that we are shutting down the async thread, so that
+  // getAsyncExecutionTarget knows not to expose/create the async thread.
+  {
+    MutexAutoLock lockedScope(mAsyncExecutionMutex);
+    NS_ENSURE_FALSE(mAsyncExecutionThreadShuttingDown, NS_ERROR_UNEXPECTED);
+    mAsyncExecutionThreadShuttingDown = true;
+  }
+
+  return NS_OK;
+}
 
-NS_IMETHODIMP
-Connection::Close()
+nsresult
+Connection::internalClose()
 {
-  if (!mDBConn)
-    return NS_ERROR_NOT_INITIALIZED;
+#ifdef DEBUG
+  // Sanity checks to make sure we are in the proper state before calling this.
+  NS_ASSERTION(mDBConn, "Database connection is already null!");
+
+  { // Make sure we have marked our async thread as shutting down.
+    MutexAutoLock lockedScope(mAsyncExecutionMutex);
+    NS_ASSERTION(mAsyncExecutionThreadShuttingDown,
+                 "Did not call setClosedState!");
+  }
+#endif
 
 #ifdef PR_LOGGING
   nsCAutoString leafName(":memory");
   if (mDatabaseFile)
       (void)mDatabaseFile->GetNativeLeafName(leafName);
   PR_LOG(gStorageLog, PR_LOG_NOTICE, ("Closing connection to '%s'",
                                       leafName.get()));
 #endif
 
-  // Flag that we are shutting down the async thread, so that
-  // getAsyncExecutionTarget knows not to expose/create the async thread.
-  {
-    nsAutoLock mutex(mAsyncExecutionMutex);
-    mAsyncExecutionThreadShuttingDown = PR_TRUE;
-  }
-  // Shutdown the async thread if it exists.  (Because we just set the flag,
-  // we are the only code that is going to be touching this variable from here
-  // on out.)
-  if (mAsyncExecutionThread) {
-    mAsyncExecutionThread->Shutdown();
-    mAsyncExecutionThread = nsnull;
-  }
-
 #ifdef DEBUG
   // Notify about any non-finalized statements.
   sqlite3_stmt *stmt = NULL;
   while ((stmt = ::sqlite3_next_stmt(mDBConn, stmt))) {
     char *msg = ::PR_smprintf("SQL statement '%s' was not finalized",
                               ::sqlite3_sql(stmt));
     NS_WARNING(msg);
     ::PR_smprintf_free(msg);
   }
 #endif
 
-  {
-    nsAutoLock mutex(mProgressHandlerMutex);
-    if (mProgressHandler)
-      ::sqlite3_progress_handler(mDBConn, 0, NULL, NULL);
-  }
-
   int srv = ::sqlite3_close(mDBConn);
   NS_ASSERTION(srv == SQLITE_OK,
                "sqlite3_close failed. There are probably outstanding statements that are listed above!");
+  mDBConn = NULL;
 
-  mDBConn = NULL;
   return convertResultCode(srv);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+//// mozIStorageConnection
+
+NS_IMETHODIMP
+Connection::Close()
+{
+  if (!mDBConn)
+    return NS_ERROR_NOT_INITIALIZED;
+
+  { // Make sure we have not executed any asynchronous statements.
+    MutexAutoLock lockedScope(mAsyncExecutionMutex);
+    NS_ENSURE_FALSE(mAsyncExecutionThread, NS_ERROR_UNEXPECTED);
+  }
+
+  nsresult rv = setClosedState();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return internalClose();
+}
+
+NS_IMETHODIMP
+Connection::AsyncClose(mozIStorageCompletionCallback *aCallback)
+{
+  if (!mDBConn)
+    return NS_ERROR_NOT_INITIALIZED;
+
+  nsCOMPtr<nsIEventTarget> asyncThread(getAsyncExecutionTarget());
+  NS_ENSURE_TRUE(asyncThread, NS_ERROR_UNEXPECTED);
+
+  nsresult rv = setClosedState();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Create our callback event if we were given a callback.
+  nsCOMPtr<nsIRunnable> completeEvent;
+  if (aCallback) {
+    completeEvent = NS_NEW_RUNNABLE_METHOD(mozIStorageCompletionCallback,
+                                           aCallback, Complete);
+    NS_ENSURE_TRUE(completeEvent, NS_ERROR_OUT_OF_MEMORY);
+  }
+
+  // Create and dispatch our close event to the background thread.
+  nsCOMPtr<nsIRunnable> closeEvent =
+    new AsyncCloseConnection(this, NS_GetCurrentThread(), completeEvent);
+  NS_ENSURE_TRUE(closeEvent, NS_ERROR_OUT_OF_MEMORY);
+
+  rv = asyncThread->Dispatch(closeEvent, NS_DISPATCH_NORMAL);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 Connection::GetConnectionReady(PRBool *_ready)
 {
   *_ready = (mDBConn != nsnull);
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- a/storage/src/mozStorageConnection.h
+++ b/storage/src/mozStorageConnection.h
@@ -95,20 +95,33 @@ public:
 
   /**
    * Mutex used by asynchronous statements to protect state.  The mutex is
    * declared on the connection object because there is no contention between
    * asynchronous statements (they are serialized on mAsyncExecutionThread).
    */
   Mutex sharedAsyncExecutionMutex;
 
+  /**
+   * Closes the SQLite database, and warns about any non-finalized statements.
+   */
+  nsresult internalClose();
+
 private:
   ~Connection();
 
   /**
+   * Sets the database into a closed state so no further actions can be
+   * performed.
+   *
+   * @note mDBConn is set to NULL in this method.
+   */
+  nsresult setClosedState();
+
+  /**
    * Describes a certain primitive type in the database.
    *
    * Possible Values Are:
    *  INDEX - To check for the existence of an index
    *  TABLE - To check for the existence of a table
    */
   enum DatabaseElementType {
     INDEX,
@@ -135,33 +148,33 @@ private:
   // Dispatch call to registered progress handler,
   // if there is one. Do nothing in other cases.
   int progressHandler();
 
   sqlite3 *mDBConn;
   nsCOMPtr<nsIFile> mDatabaseFile;
 
   /**
-   * Protects access to mAsyncExecutionThread.
+   * Protects access to mAsyncExecutionThread and mPendingStatements.
    */
-  PRLock *mAsyncExecutionMutex;
+  Mutex mAsyncExecutionMutex;
 
   /**
    * Lazily created thread for asynchronous statement execution.  Consumers
    * should use getAsyncExecutionTarget rather than directly accessing this
    * field.
    */
   nsCOMPtr<nsIThread> mAsyncExecutionThread;
   /**
    * Set to true by Close() prior to actually shutting down the thread.  This
    * lets getAsyncExecutionTarget() know not to hand out any more thread
    * references (or to create the thread in the first place).  This variable
    * should be accessed while holding the mAsyncExecutionMutex.
    */
-  PRBool mAsyncExecutionThreadShuttingDown;
+  bool mAsyncExecutionThreadShuttingDown;
 
   PRLock *mTransactionMutex;
   PRBool mTransactionInProgress;
 
   PRLock *mFunctionsMutex;
   nsInterfaceHashtable<nsCStringHashKey, nsISupports> mFunctions;
 
   PRLock *mProgressHandlerMutex;
--- a/storage/test/unit/test_connection_executeAsync.js
+++ b/storage/test/unit/test_connection_executeAsync.js
@@ -231,36 +231,111 @@ function test_multiple_bindings_on_state
 
       // Run the next test.
       run_next_test();
     }
   });
   stmts.forEach(function(stmt) stmt.finalize());
 }
 
+function test_asyncClose_does_not_complete_before_statements()
+{
+  let stmt = createStatement("SELECT * FROM sqlite_master");
+  let executed = false;
+  stmt.executeAsync({
+    handleResult: function(aResultSet)
+    {
+    },
+    handleError: function(aError)
+    {
+      print("Error code " + aError.result + " with message '" +
+            aError.message + "' returned.");
+      do_throw("Unexpected error!");
+    },
+    handleCompletion: function(aReason)
+    {
+      print("handleCompletion(" + aReason +
+            ") for test_asyncClose_does_not_complete_before_statements");
+      do_check_eq(Ci.mozIStorageStatementCallback.REASON_FINISHED, aReason);
+      executed = true;
+    }
+  });
+  stmt.finalize();
+
+  getOpenedDatabase().asyncClose(function() {
+    // Ensure that the statement executed to completion.
+    do_check_true(executed);
+
+    // Reset gDBConn so that later tests will get a new connection object.
+    gDBConn = null;
+    run_next_test();
+  });
+}
+
+function test_asyncClose_does_not_throw_no_callback()
+{
+  getOpenedDatabase().asyncClose();
+
+  // Reset gDBConn so that later tests will get a new connection object.
+  gDBConn = null;
+  run_next_test();
+}
+
+function test_double_asyncClose_throws()
+{
+  let conn = getOpenedDatabase();
+  conn.asyncClose();
+  try {
+    conn.asyncClose();
+    do_throw("should have thrown");
+  }
+  catch (e) {
+    do_check_eq(e.result, Cr.NS_ERROR_UNEXPECTED);
+  }
+
+  // Reset gDBConn so that later tests will get a new connection object.
+  gDBConn = null;
+  run_next_test();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// Test Runner
 
 let tests =
 [
   test_create_and_add,
   test_transaction_created,
   test_multiple_bindings_on_statements,
+  test_asyncClose_does_not_complete_before_statements,
+  test_asyncClose_does_not_throw_no_callback,
+  test_double_asyncClose_throws,
 ];
 let index = 0;
 
 function run_next_test()
 {
-  if (index < tests.length) {
-    do_test_pending();
-    print("Running the next test: " + tests[index].name);
-    tests[index++]();
+  function _run_next_test() {
+    if (index < tests.length) {
+      do_test_pending();
+      print("Running the next test: " + tests[index].name);
+
+      // Asynchronous tests means that exceptions don't kill the test.
+      try {
+        tests[index++]();
+      }
+      catch (e) {
+        do_throw(e);
+      }
+    }
+
+    do_test_finished();
   }
 
-  do_test_finished();
+  // For saner stacks, we execute this code RSN.
+  do_execute_soon(_run_next_test);
 }
 
 function run_test()
 {
   cleanup();
 
   do_test_pending();
   run_next_test();
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -44,192 +44,271 @@ function test_connectionReady_open()
 {
   // there doesn't seem to be a way for the connection to not be ready (unless
   // we close it with mozIStorageConnection::Close(), but we don't for this).
   // It can only fail if GetPath fails on the database file, or if we run out
   // of memory trying to use an in-memory database
 
   var msc = getOpenedDatabase();
   do_check_true(msc.connectionReady);
+  run_next_test();
 }
 
 function test_connectionReady_closed()
 {
   // This also tests mozIStorageConnection::Close()
 
   var msc = getOpenedDatabase();
   msc.close();
   do_check_false(msc.connectionReady);
   gDBConn = null; // this is so later tests don't start to fail.
+  run_next_test();
 }
 
 function test_databaseFile()
 {
   var msc = getOpenedDatabase();
   do_check_true(getTestDB().equals(msc.databaseFile));
+  run_next_test();
 }
 
 function test_tableExists_not_created()
 {
   var msc = getOpenedDatabase();
   do_check_false(msc.tableExists("foo"));
+  run_next_test();
 }
 
 function test_indexExists_not_created()
 {
   var msc = getOpenedDatabase();
   do_check_false(msc.indexExists("foo"));
+  run_next_test();
 }
 
 function test_createTable_not_created()
 {
   var msc = getOpenedDatabase();
   msc.createTable("test", "id INTEGER PRIMARY KEY, name TEXT");
   do_check_true(msc.tableExists("test"));
+  run_next_test();
 }
 
 function test_indexExists_created()
 {
   var msc = getOpenedDatabase();
   msc.executeSimpleSQL("CREATE INDEX name_ind ON test (name)");
   do_check_true(msc.indexExists("name_ind"));
+  run_next_test();
 }
 
 function test_createTable_already_created()
 {
   var msc = getOpenedDatabase();
   do_check_true(msc.tableExists("test"));
   try {
     msc.createTable("test", "id INTEGER PRIMARY KEY, name TEXT");
     do_throw("We shouldn't get here!");
   } catch (e) {
     do_check_eq(Cr.NS_ERROR_FAILURE, e.result);
   }
+  run_next_test();
 }
 
 function test_lastInsertRowID()
 {
   var msc = getOpenedDatabase();
   msc.executeSimpleSQL("INSERT INTO test (name) VALUES ('foo')");
   do_check_eq(1, msc.lastInsertRowID);
+  run_next_test();
 }
 
 function test_transactionInProgress_no()
 {
   var msc = getOpenedDatabase();
   do_check_false(msc.transactionInProgress);
+  run_next_test();
 }
 
 function test_transactionInProgress_yes()
 {
   var msc = getOpenedDatabase();
   msc.beginTransaction();
   do_check_true(msc.transactionInProgress);
   msc.commitTransaction();
   do_check_false(msc.transactionInProgress);
 
   msc.beginTransaction();
   do_check_true(msc.transactionInProgress);
   msc.rollbackTransaction();
   do_check_false(msc.transactionInProgress);
+  run_next_test();
 }
 
 function test_commitTransaction_no_transaction()
 {
   var msc = getOpenedDatabase();
   do_check_false(msc.transactionInProgress);
   try {
     msc.commitTransaction();
     do_throw("We should not get here!");
   } catch (e) {
     do_check_eq(Cr.NS_ERROR_FAILURE, e.result);
   }
+  run_next_test();
 }
 
 function test_rollbackTransaction_no_transaction()
 {
   var msc = getOpenedDatabase();
   do_check_false(msc.transactionInProgress);
   try {
     msc.rollbackTransaction();
     do_throw("We should not get here!");
   } catch (e) {
     do_check_eq(Cr.NS_ERROR_FAILURE, e.result);
   }
+  run_next_test();
 }
 
 function test_get_schemaVersion_not_set()
 {
   do_check_eq(0, getOpenedDatabase().schemaVersion);
+  run_next_test();
 }
 
 function test_set_schemaVersion()
 {
   var msc = getOpenedDatabase();
   const version = 1;
   msc.schemaVersion = version;
   do_check_eq(version, msc.schemaVersion);
+  run_next_test();
 }
 
 function test_set_schemaVersion_same()
 {
   var msc = getOpenedDatabase();
   const version = 1;
   msc.schemaVersion = version; // should still work ok
   do_check_eq(version, msc.schemaVersion);
+  run_next_test();
 }
 
 function test_set_schemaVersion_negative()
 {
   var msc = getOpenedDatabase();
   const version = -1;
   msc.schemaVersion = version;
   do_check_eq(version, msc.schemaVersion);
+  run_next_test();
 }
 
 function test_createTable(){
   var temp = getTestDB().parent;
   temp.append("test_db_table");
   try {
     var con = getService().openDatabase(temp);
     con.createTable("a","");
   } catch (e) {
     if (temp.exists()) try {
       temp.remove(false);
     } catch (e2) {}
     do_check_true(e.result==Cr.NS_ERROR_NOT_INITIALIZED ||
                   e.result==Cr.NS_ERROR_FAILURE);
   }
+  run_next_test();
 }
 
 function test_defaultSynchronousAtNormal()
 {
   var msc = getOpenedDatabase();
   var stmt = createStatement("PRAGMA synchronous;");
   try {
     stmt.executeStep();
     do_check_eq(1, stmt.getInt32(0));
   }
   finally {
     stmt.reset();
     stmt.finalize();
   }
+  run_next_test();
 }
 
-function test_close_succeeds_with_finalized_async_statement()
+function test_close_does_not_spin_event_loop()
 {
+  // We want to make sure that the event loop on the calling thread does not
+  // spin when close is called.
+  let event = {
+    ran: false,
+    run: function()
+    {
+      this.ran = true;
+    },
+  };
+
+  // Post the event before we call close, so it would run if the event loop was
+  // spun during close.
+  let thread = Cc["@mozilla.org/thread-manager;1"].
+               getService(Ci.nsIThreadManager).
+               currentThread;
+  thread.dispatch(event, Ci.nsIThread.DISPATCH_NORMAL);
+
+  // Sanity check, then close the database.  Afterwards, we should not have ran!
+  do_check_false(event.ran);
+  getOpenedDatabase().close();
+  do_check_false(event.ran);
+
+  // Reset gDBConn so that later tests will get a new connection object.
+  gDBConn = null;
+  run_next_test();
+}
+
+function test_asyncClose_succeeds_with_finalized_async_statement()
+{
+  // XXX this test isn't perfect since we can't totally control when events will
+  //     run.  If this paticular function fails randomly, it means we have a
+  //     real bug.
+
   // We want to make sure we create a cached async statement to make sure that
   // when we finalize our statement, we end up finalizing the async one too so
   // close will succeed.
   let stmt = createStatement("SELECT * FROM test");
   stmt.executeAsync();
   stmt.finalize();
 
-  // Cleanup calls close, as well as removes the database file.
-  cleanup();
+  getOpenedDatabase().asyncClose(function() {
+    // Reset gDBConn so that later tests will get a new connection object.
+    gDBConn = null;
+    run_next_test();
+  });
+}
+
+function test_close_fails_with_async_statement_ran()
+{
+  let stmt = createStatement("SELECT * FROM test");
+  stmt.executeAsync();
+  stmt.finalize();
+
+  let db = getOpenedDatabase();
+  try {
+    db.close();
+    do_throw("should have thrown");
+  }
+  catch (e) {
+    do_check_eq(e.result, Cr.NS_ERROR_UNEXPECTED);
+  }
+  finally {
+    // Clean up after ourselves.
+    db.asyncClose(function() {
+      // Reset gDBConn so that later tests will get a new connection object.
+      gDBConn = null;
+      run_next_test();
+    });
+  }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //// Test Runner
 
 var tests = [
   test_connectionReady_open,
   test_connectionReady_closed,
@@ -245,18 +324,44 @@ var tests = [
   test_commitTransaction_no_transaction,
   test_rollbackTransaction_no_transaction,
   test_get_schemaVersion_not_set,
   test_set_schemaVersion,
   test_set_schemaVersion_same,
   test_set_schemaVersion_negative,
   test_createTable,
   test_defaultSynchronousAtNormal,
-  test_close_succeeds_with_finalized_async_statement,
+  test_close_does_not_spin_event_loop, // must be ran before executeAsync tests
+  test_asyncClose_succeeds_with_finalized_async_statement,
+  test_close_fails_with_async_statement_ran,
 ];
+let index = 0;
+
+function run_next_test()
+{
+  function _run_next_test() {
+    if (index < tests.length) {
+      do_test_pending();
+      print("Running the next test: " + tests[index].name);
+
+      // Asynchronous tests means that exceptions don't kill the test.
+      try {
+        tests[index++]();
+      }
+      catch (e) {
+        do_throw(e);
+      }
+    }
+
+    do_test_finished();
+  }
+
+  // For saner stacks, we execute this code RSN.
+  do_execute_soon(_run_next_test);
+}
 
 function run_test()
 {
-  for (var i = 0; i < tests.length; i++)
-    tests[i]();
-    
   cleanup();
+
+  do_test_pending();
+  run_next_test();
 }