Bug 1090961 - Enqueue Sqlite.jsm transactions. r=Yoric
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 11 Dec 2014 15:50:51 +0100
changeset 244972 00b588dac8bc373f019a400baa894808721d7fc2
parent 244971 a8f9ed3d7554f4eb51cf6f0f6295f00dc22bc6a1
child 244973 c677f867a18a1c2627078263157848fb3b198406
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs1090961
milestone37.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 1090961 - Enqueue Sqlite.jsm transactions. r=Yoric
toolkit/modules/Sqlite.jsm
toolkit/modules/tests/xpcshell/test_sqlite.js
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -5,38 +5,43 @@
 "use strict";
 
 this.EXPORTED_SYMBOLS = [
   "Sqlite",
 ];
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
-Cu.import("resource://gre/modules/XPCOMUtils.jsm", this);
+// The time to wait before considering a transaction stuck and rejecting it.
+const TRANSACTIONS_QUEUE_TIMEOUT_MS = 120000 // 2 minutes
+
+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
+Cu.import("resource://gre/modules/Timer.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "AsyncShutdown",
                                   "resource://gre/modules/AsyncShutdown.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "Promise",
-                                  "resource://gre/modules/Promise.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Log",
                                   "resource://gre/modules/Log.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "CommonUtils",
                                   "resource://services-common/utils.js");
 XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
                                   "resource://gre/modules/FileUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
 XPCOMUtils.defineLazyServiceGetter(this, "FinalizationWitnessService",
                                    "@mozilla.org/toolkit/finalizationwitness;1",
                                    "nsIFinalizationWitnessService");
-
+XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
+                                  "resource://gre/modules/PromiseUtils.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "console",
+                                  "resource://gre/modules/devtools/Console.jsm");
 
 // Counts the number of created connections per database basename(). This is
 // used for logging to distinguish connection instances.
 let connectionCounters = new Map();
 
 // Tracks identifiers of wrapped connections, that are Storage connections
 // opened through mozStorage and then wrapped by Sqlite.jsm to use its syntactic
 // sugar API.  Since these connections have an unknown origin, we use this set
@@ -186,17 +191,17 @@ XPCOMUtils.defineLazyGetter(this, "Barri
  * OpenedConnection. When the witness detects a garbage collection,
  * this object can be used to close the connection.
  *
  * This object contains more methods than just `close`.  When
  * OpenedConnection needs to use the methods in this object, it will
  * dispatch its method calls here.
  */
 function ConnectionData(connection, identifier, options={}) {
-  this._log = Log.repository.getLoggerWithMessagePrefix("Sqlite.Connection." +
+  this._log = Log.repository.getLoggerWithMessagePrefix("Sqlite.Connection",
                                                         identifier + ": ");
   this._log.info("Opened");
 
   this._dbConn = connection;
 
   // This is a unique identifier for the connection, generated through
   // getIdentifierByPath.  It may be used for logging or as a key in Maps.
   this._identifier = identifier;
@@ -209,37 +214,42 @@ function ConnectionData(connection, iden
 
   // A map from statement index to mozIStoragePendingStatement, to allow for
   // canceling prior to finalizing the mozIStorageStatements.
   this._pendingStatements = new Map();
 
   // Increments for each executed statement for the life of the connection.
   this._statementCounter = 0;
 
-  this._inProgressTransaction = null;
+  this._hasInProgressTransaction = false;
+  // Manages a chain of transactions promises, so that new transactions
+  // always happen in queue to the previous ones.  It never rejects.
+  this._transactionQueue = Promise.resolve();
 
   this._idleShrinkMS = options.shrinkMemoryOnConnectionIdleMS;
   if (this._idleShrinkMS) {
     this._idleShrinkTimer = Cc["@mozilla.org/timer;1"]
                               .createInstance(Ci.nsITimer);
     // We wait for the first statement execute to start the timer because
     // shrinking now would not do anything.
   }
 
-  this._deferredClose = Promise.defer();
+  // Deferred whose promise is resolved when the connection closing procedure
+  // is complete.
+  this._deferredClose = PromiseUtils.defer();
   this._closeRequested = false;
 
   Barriers.connections.client.addBlocker(
     this._identifier + ": waiting for shutdown",
     this._deferredClose.promise,
     () =>  ({
       identifier: this._identifier,
       isCloseRequested: this._closeRequested,
       hasDbConn: !!this._dbConn,
-      hasInProgressTransaction: !!this._inProgressTransaction,
+      hasInProgressTransaction: this._hasInProgressTransaction,
       pendingStatements: this._pendingStatements.size,
       statementCounter: this._statementCounter,
     })
   );
 }
 
 /**
  * Map of connection identifiers to ConnectionData objects
@@ -263,33 +273,26 @@ ConnectionData.prototype = Object.freeze
 
     this._log.debug("Request to close connection.");
     this._clearIdleShrinkTimer();
 
     // We need to take extra care with transactions during shutdown.
     //
     // If we don't have a transaction in progress, we can proceed with shutdown
     // immediately.
-    if (!this._inProgressTransaction) {
-      this._finalize(this._deferredClose);
-      return this._deferredClose.promise;
+    if (!this._hasInProgressTransaction) {
+      return this._finalize();
     }
 
-    // Else if we do have a transaction in progress, we forcefully roll it
-    // back. This is an async task, so we wait on it to finish before
-    // performing finalization.
+    // If instead we do have a transaction in progress, it might be rollback-ed
+    // automaticall by closing the connection.  Regardless, we wait for its
+    // completion, next enqueued transactions will be rejected.
     this._log.warn("Transaction in progress at time of close. Rolling back.");
 
-    let onRollback = this._finalize.bind(this, this._deferredClose);
-
-    this.execute("ROLLBACK TRANSACTION").then(onRollback, onRollback);
-    this._inProgressTransaction.reject(new Error("Connection being closed."));
-    this._inProgressTransaction = null;
-
-    return this._deferredClose.promise;
+    return this._transactionQueue.then(() => this._finalize());
   },
 
   clone: function (readOnly=false) {
     this.ensureOpen();
 
     this._log.debug("Request to clone connection.");
 
     let options = {
@@ -297,17 +300,17 @@ ConnectionData.prototype = Object.freeze
       readOnly: readOnly,
     };
     if (this._idleShrinkMS)
       options.shrinkMemoryOnConnectionIdleMS = this._idleShrinkMS;
 
     return cloneStorageConnection(options);
   },
 
-  _finalize: function (deferred) {
+  _finalize: function () {
     this._log.debug("Finalizing connection.");
     // Cancel any pending statements.
     for (let [k, statement] of this._pendingStatements) {
       statement.cancel();
     }
     this._pendingStatements.clear();
 
     // We no longer need to track these.
@@ -330,26 +333,27 @@ ConnectionData.prototype = Object.freeze
 
     // We must always close the connection at the Sqlite.jsm-level, not
     // necessarily at the mozStorage-level.
     let markAsClosed = () => {
       this._log.info("Closed");
       this._dbConn = null;
       // Now that the connection is closed, no need to keep
       // a blocker for Barriers.connections.
-      Barriers.connections.client.removeBlocker(deferred.promise);
-      deferred.resolve();
+      Barriers.connections.client.removeBlocker(this._deferredClose.promise);
+      this._deferredClose.resolve();
     }
     if (wrappedConnections.has(this._identifier)) {
       wrappedConnections.delete(this._identifier);
       markAsClosed();
     } else {
       this._log.debug("Calling asyncClose().");
       this._dbConn.asyncClose(markAsClosed);
     }
+    return this._deferredClose.promise;
   },
 
   executeCached: function (sql, params=null, onRow=null) {
     this.ensureOpen();
 
     if (!sql) {
       throw new Error("sql argument is empty.");
     }
@@ -357,35 +361,33 @@ ConnectionData.prototype = Object.freeze
     let statement = this._cachedStatements.get(sql);
     if (!statement) {
       statement = this._dbConn.createAsyncStatement(sql);
       this._cachedStatements.set(sql, statement);
     }
 
     this._clearIdleShrinkTimer();
 
-    let deferred = Promise.defer();
-
-    try {
-      this._executeStatement(sql, statement, params, onRow).then(
-        result => {
-          this._startIdleShrinkTimer();
-          deferred.resolve(result);
-        },
-        error => {
-          this._startIdleShrinkTimer();
-          deferred.reject(error);
-        }
-      );
-    } catch (ex) {
-      this._startIdleShrinkTimer();
-      throw ex;
-    }
-
-    return deferred.promise;
+    return new Promise((resolve, reject) => {
+      try {
+        this._executeStatement(sql, statement, params, onRow).then(
+          result => {
+            this._startIdleShrinkTimer();
+            resolve(result);
+          },
+          error => {
+            this._startIdleShrinkTimer();
+            reject(error);
+          }
+        );
+      } catch (ex) {
+        this._startIdleShrinkTimer();
+        throw ex;
+      }
+    });
   },
 
   execute: function (sql, params=null, onRow=null) {
     if (typeof(sql) != "string") {
       throw new Error("Must define SQL to execute as a string: " + sql);
     }
 
     this.ensureOpen();
@@ -397,113 +399,142 @@ ConnectionData.prototype = Object.freeze
     this._clearIdleShrinkTimer();
 
     let onFinished = () => {
       this._anonymousStatements.delete(index);
       statement.finalize();
       this._startIdleShrinkTimer();
     };
 
-    let deferred = Promise.defer();
-
-    try {
-      this._executeStatement(sql, statement, params, onRow).then(
-        rows => {
-          onFinished();
-          deferred.resolve(rows);
-        },
-        error => {
-          onFinished();
-          deferred.reject(error);
-        }
-      );
-    } catch (ex) {
-      onFinished();
-      throw ex;
-    }
-
-    return deferred.promise;
+    return new Promise((resolve, reject) => {
+      try {
+        this._executeStatement(sql, statement, params, onRow).then(
+          rows => {
+            onFinished();
+            resolve(rows);
+          },
+          error => {
+            onFinished();
+            reject(error);
+          }
+        );
+      } catch (ex) {
+        onFinished();
+        throw ex;
+      }
+    });
   },
 
   get transactionInProgress() {
-    return this._open && !!this._inProgressTransaction;
+    return this._open && this._hasInProgressTransaction;
   },
 
   executeTransaction: function (func, type) {
     this.ensureOpen();
 
-    if (this._inProgressTransaction) {
-      throw new Error("A transaction is already active. Only one transaction " +
-                      "can be active at a time.");
-    }
-
     this._log.debug("Beginning transaction");
-    let deferred = Promise.defer();
-    this._inProgressTransaction = deferred;
-    Task.spawn(function doTransaction() {
-      // It's tempting to not yield here and rely on the implicit serial
-      // execution of issued statements. However, the yield serves an important
-      // purpose: catching errors in statement execution.
-      yield this.execute("BEGIN " + type + " TRANSACTION");
 
-      let result;
-      try {
-        result = yield Task.spawn(func);
-      } catch (ex) {
-        // It's possible that a request to close the connection caused the
-        // error.
-        // Assertion: close() will unset
-        // this._inProgressTransaction when called.
-        if (!this._inProgressTransaction) {
-          this._log.warn("Connection was closed while performing transaction. " +
-                         "Received error should be due to closed connection: " +
-                         CommonUtils.exceptionStr(ex));
-          throw ex;
-        }
-
-        this._log.warn("Error during transaction. Rolling back: " +
-                       CommonUtils.exceptionStr(ex));
-        try {
-          yield this.execute("ROLLBACK TRANSACTION");
-        } catch (inner) {
-          this._log.warn("Could not roll back transaction. This is weird: " +
-                         CommonUtils.exceptionStr(inner));
-        }
-
-        throw ex;
+    let promise = this._transactionQueue.then(() => {
+      if (this._closeRequested) {
+        throw new Error("Transaction canceled due to a closed connection.");
       }
 
-      // See comment above about connection being closed during transaction.
-      if (!this._inProgressTransaction) {
-        this._log.warn("Connection was closed while performing transaction. " +
-                       "Unable to commit.");
-        throw new Error("Connection closed before transaction committed.");
-      }
+      let transactionPromise = Task.spawn(function* () {
+        // At this point we should never have an in progress transaction, since
+        // they are enqueued.
+        if (this._hasInProgressTransaction) {
+          console.error("Unexpected transaction in progress when trying to start a new one.");
+        }
+        this._hasInProgressTransaction = true;
+        try {
+          // We catch errors in statement execution to detect nested transactions.
+          try {
+            yield this.execute("BEGIN " + type + " TRANSACTION");
+          } catch (ex) {
+            // Unfortunately, if we are wrapping an existing connection, a
+            // transaction could have been started by a client of the same
+            // connection that doesn't use Sqlite.jsm (e.g. C++ consumer).
+            // The best we can do is proceed without a transaction and hope
+            // things won't break.
+            if (wrappedConnections.has(this._identifier)) {
+              this._log.warn("A new transaction could not be started cause the wrapped connection had one in progress: " +
+                             CommonUtils.exceptionStr(ex));
+              // Unmark the in progress transaction, since it's managed by
+              // some other non-Sqlite.jsm client.  See the comment above.
+              this._hasInProgressTransaction = false;
+            } else {
+              this._log.warn("A transaction was already in progress, likely a nested transaction: " +
+                             CommonUtils.exceptionStr(ex));
+              throw ex;
+            }
+          }
 
-      try {
-        yield this.execute("COMMIT TRANSACTION");
-      } catch (ex) {
-        this._log.warn("Error committing transaction: " +
-                       CommonUtils.exceptionStr(ex));
-        throw ex;
-      }
+          let result;
+          try {
+            result = yield Task.spawn(func);
+          } catch (ex) {
+            // It's possible that the exception has been caused by trying to
+            // close the connection in the middle of a transaction.
+            if (this._closeRequested) {
+              this._log.warn("Connection closed while performing a transaction: " +
+                             CommonUtils.exceptionStr(ex));
+            } else {
+              this._log.warn("Error during transaction. Rolling back: " +
+                             CommonUtils.exceptionStr(ex));
+              // If we began a transaction, we must rollback it.
+              if (this._hasInProgressTransaction) {
+                try {
+                  yield this.execute("ROLLBACK TRANSACTION");
+                } catch (inner) {
+                  this._log.warn("Could not roll back transaction: " +
+                                 CommonUtils.exceptionStr(inner));
+                }
+              }
+            }
+            // Rethrow the exception.
+            throw ex;
+          }
+
+          // See comment above about connection being closed during transaction.
+          if (this._closeRequested) {
+            this._log.warn("Connection closed before committing the transaction.");
+            throw new Error("Connection closed before committing the transaction.");
+          }
 
-      throw new Task.Result(result);
-    }.bind(this)).then(
-      function onSuccess(result) {
-        this._inProgressTransaction = null;
-        deferred.resolve(result);
-      }.bind(this),
-      function onError(error) {
-        this._inProgressTransaction = null;
-        deferred.reject(error);
-      }.bind(this)
-    );
+          // If we began a transaction, we must commit it.
+          if (this._hasInProgressTransaction) {
+            try {
+              yield this.execute("COMMIT TRANSACTION");
+            } catch (ex) {
+              this._log.warn("Error committing transaction: " +
+                             CommonUtils.exceptionStr(ex));
+              throw ex;
+            }
+          }
 
-    return deferred.promise;
+          return result;
+        } finally {
+          this._hasInProgressTransaction = false;
+        }
+      }.bind(this));
+
+      // If a transaction yields on a never resolved promise, or is mistakenly
+      // nested, it could hang the transactions queue forever.  Thus we timeout
+      // the execution after a meaningful amount of time, to ensure in any case
+      // we'll proceed after a while.
+      let timeoutPromise = new Promise((resolve, reject) => {
+        setTimeout(() => reject(new Error("Transaction timeout, most likely caused by unresolved pending work.")),
+                   TRANSACTIONS_QUEUE_TIMEOUT_MS);
+      });
+      return Promise.race([transactionPromise, timeoutPromise]);
+    });
+    // Atomically update the queue before anyone else has a chance to enqueue
+    // further transactions.
+    this._transactionQueue = promise.catch(ex => { console.error(ex) });
+    return promise;
   },
 
   shrinkMemory: function () {
     this._log.info("Shrinking memory usage.");
     let onShrunk = this._clearIdleShrinkTimer.bind(this);
     return this.execute("PRAGMA shrink_memory").then(onShrunk, onShrunk);
   },
 
@@ -570,17 +601,17 @@ ConnectionData.prototype = Object.freeze
     if (onRow && typeof(onRow) != "function") {
       throw new Error("onRow must be a function. Got: " + onRow);
     }
 
     this._bindParameters(statement, params);
 
     let index = this._statementCounter++;
 
-    let deferred = Promise.defer();
+    let deferred = PromiseUtils.defer();
     let userCancelled = false;
     let errors = [];
     let rows = [];
     let handledRow = false;
 
     // Don't incur overhead for serializing params unless the messages go
     // somewhere.
     if (this._log.level <= Log.Level.Trace) {
@@ -733,17 +764,16 @@ function openConnection(options) {
   if (!options.path) {
     throw new Error("path not specified in connection options.");
   }
 
   if (isClosed) {
     throw new Error("Sqlite.jsm has been shutdown. Cannot open connection to: " + options.path);
   }
 
-
   // Retains absolute paths and normalizes relative as relative to profile.
   let path = OS.Path.join(OS.Constants.Path.profileDir, options.path);
 
   let sharedMemoryCache = "sharedMemoryCache" in options ?
                             options.sharedMemoryCache : true;
 
   let openedOptions = {};
 
@@ -756,40 +786,41 @@ function openConnection(options) {
     openedOptions.shrinkMemoryOnConnectionIdleMS =
       options.shrinkMemoryOnConnectionIdleMS;
   }
 
   let file = FileUtils.File(path);
   let identifier = getIdentifierByPath(path);
 
   log.info("Opening database: " + path + " (" + identifier + ")");
-  let deferred = Promise.defer();
-  let dbOptions = null;
-  if (!sharedMemoryCache) {
-    dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
-      createInstance(Ci.nsIWritablePropertyBag);
-    dbOptions.setProperty("shared", false);
-  }
-  Services.storage.openAsyncDatabase(file, dbOptions, function(status, connection) {
-    if (!connection) {
-      log.warn("Could not open connection: " + status);
-      deferred.reject(new Error("Could not open connection: " + status));
-      return;
+
+  return new Promise((resolve, reject) => {
+    let dbOptions = null;
+    if (!sharedMemoryCache) {
+      dbOptions = Cc["@mozilla.org/hash-property-bag;1"].
+        createInstance(Ci.nsIWritablePropertyBag);
+      dbOptions.setProperty("shared", false);
     }
-    log.info("Connection opened");
-    try {
-      deferred.resolve(
-        new OpenedConnection(connection.QueryInterface(Ci.mozIStorageAsyncConnection),
-                            identifier, openedOptions));
-    } catch (ex) {
-      log.warn("Could not open database: " + CommonUtils.exceptionStr(ex));
-      deferred.reject(ex);
-    }
+    Services.storage.openAsyncDatabase(file, dbOptions, (status, connection) => {
+      if (!connection) {
+        log.warn("Could not open connection: " + status);
+        reject(new Error("Could not open connection: " + status));
+        return;
+      }
+      log.info("Connection opened");
+      try {
+        resolve(
+          new OpenedConnection(connection.QueryInterface(Ci.mozIStorageAsyncConnection),
+                              identifier, openedOptions));
+      } catch (ex) {
+        log.warn("Could not open database: " + CommonUtils.exceptionStr(ex));
+        reject(ex);
+      }
+    });
   });
-  return deferred.promise;
 }
 
 /**
  * Creates a clone of an existing and open Storage connection.  The clone has
  * the same underlying characteristics of the original connection and is
  * returned in form of an OpenedConnection handle.
  *
  * The following parameters can control the cloned connection:
@@ -841,33 +872,33 @@ function cloneStorageConnection(options)
     openedOptions.shrinkMemoryOnConnectionIdleMS =
       options.shrinkMemoryOnConnectionIdleMS;
   }
 
   let path = source.databaseFile.path;
   let identifier = getIdentifierByPath(path);
 
   log.info("Cloning database: " + path + " (" + identifier + ")");
-  let deferred = Promise.defer();
 
-  source.asyncClone(!!options.readOnly, (status, connection) => {
-    if (!connection) {
-      log.warn("Could not clone connection: " + status);
-      deferred.reject(new Error("Could not clone connection: " + status));
-    }
-    log.info("Connection cloned");
-    try {
-      let conn = connection.QueryInterface(Ci.mozIStorageAsyncConnection);
-      deferred.resolve(new OpenedConnection(conn, identifier, openedOptions));
-    } catch (ex) {
-      log.warn("Could not clone database: " + CommonUtils.exceptionStr(ex));
-      deferred.reject(ex);
-    }
+  return new Promise((resolve, reject) => {
+    source.asyncClone(!!options.readOnly, (status, connection) => {
+      if (!connection) {
+        log.warn("Could not clone connection: " + status);
+        reject(new Error("Could not clone connection: " + status));
+      }
+      log.info("Connection cloned");
+      try {
+        let conn = connection.QueryInterface(Ci.mozIStorageAsyncConnection);
+        resolve(new OpenedConnection(conn, identifier, openedOptions));
+      } catch (ex) {
+        log.warn("Could not clone database: " + CommonUtils.exceptionStr(ex));
+        reject(ex);
+      }
+    });
   });
-  return deferred.promise;
 }
 
 /**
  * Wraps an existing and open Storage connection with Sqlite.jsm API.  The
  * wrapped connection clone has the same underlying characteristics of the
  * original connection and is returned in form of an OpenedConnection handle.
  *
  * Clients are responsible for closing both the Sqlite.jsm wrapper and the
@@ -1150,16 +1181,31 @@ OpenedConnection.prototype = Object.free
    */
   get transactionInProgress() {
     return this._connectionData.transactionInProgress;
   },
 
   /**
    * Perform a transaction.
    *
+   * *****************************************************************************
+   * YOU SHOULD _NEVER_ NEST executeTransaction CALLS FOR ANY REASON, NOR
+   * DIRECTLY, NOR THROUGH OTHER PROMISES.
+   * FOR EXAMPLE, NEVER DO SOMETHING LIKE:
+   *   yield executeTransaction(function* () {
+   *     ...some_code...
+   *     yield executeTransaction(function* () { // WRONG!
+   *       ...some_code...
+   *     })
+   *     yield someCodeThatExecuteTransaction(); // WRONG!
+   *     yield neverResolvedPromise; // WRONG!
+   *   });
+   * NESTING CALLS WILL BLOCK ANY FUTURE TRANSACTION UNTIL A TIMEOUT KICKS IN.
+   * *****************************************************************************
+   *
    * A transaction is specified by a user-supplied function that is a
    * generator function which can be used by Task.jsm's Task.spawn(). The
    * function receives this connection instance as its argument.
    *
    * The supplied function is expected to yield promises. These are often
    * promises created by calling `execute` and `executeCached`. If the
    * generator is exhausted without any errors being thrown, the
    * transaction is committed. If an error occurs, the transaction is
--- a/toolkit/modules/tests/xpcshell/test_sqlite.js
+++ b/toolkit/modules/tests/xpcshell/test_sqlite.js
@@ -45,77 +45,77 @@ function getConnection(dbName, extraOpti
   let options = {path: path};
   for (let [k, v] in Iterator(extraOptions)) {
     options[k] = v;
   }
 
   return Sqlite.openConnection(options);
 }
 
-function getDummyDatabase(name, extraOptions={}) {
+function* getDummyDatabase(name, extraOptions={}) {
   const TABLES = {
     dirs: "id INTEGER PRIMARY KEY AUTOINCREMENT, path TEXT",
     files: "id INTEGER PRIMARY KEY AUTOINCREMENT, dir_id INTEGER, path TEXT",
   };
 
   let c = yield getConnection(name, extraOptions);
   c._initialStatementCount = 0;
 
   for (let [k, v] in Iterator(TABLES)) {
     yield c.execute("CREATE TABLE " + k + "(" + v + ")");
     c._initialStatementCount++;
   }
 
-  throw new Task.Result(c);
+  return c;
 }
 
-function getDummyTempDatabase(name, extraOptions={}) {
+function* getDummyTempDatabase(name, extraOptions={}) {
   const TABLES = {
     dirs: "id INTEGER PRIMARY KEY AUTOINCREMENT, path TEXT",
     files: "id INTEGER PRIMARY KEY AUTOINCREMENT, dir_id INTEGER, path TEXT",
   };
 
   let c = yield getConnection(name, extraOptions);
   c._initialStatementCount = 0;
 
   for (let [k, v] in Iterator(TABLES)) {
     yield c.execute("CREATE TEMP TABLE " + k + "(" + v + ")");
     c._initialStatementCount++;
   }
 
-  throw new Task.Result(c);
+  return c;
 }
 
 function run_test() {
   Cu.import("resource://testing-common/services/common/logging.js");
   initTestLogging("Trace");
 
   run_next_test();
 }
 
-add_task(function test_open_normal() {
+add_task(function* test_open_normal() {
   let c = yield Sqlite.openConnection({path: "test_open_normal.sqlite"});
   yield c.close();
 });
 
-add_task(function test_open_unshared() {
+add_task(function* test_open_unshared() {
   let path = OS.Path.join(OS.Constants.Path.profileDir, "test_open_unshared.sqlite");
 
   let c = yield Sqlite.openConnection({path: path, sharedMemoryCache: false});
   yield c.close();
 });
 
-add_task(function test_get_dummy_database() {
+add_task(function* test_get_dummy_database() {
   let db = yield getDummyDatabase("get_dummy_database");
 
   do_check_eq(typeof(db), "object");
   yield db.close();
 });
 
-add_task(function test_schema_version() {
+add_task(function* test_schema_version() {
   let db = yield getDummyDatabase("schema_version");
 
   let version = yield db.getSchemaVersion();
   do_check_eq(version, 0);
 
   db.setSchemaVersion(14);
   version = yield db.getSchemaVersion();
   do_check_eq(version, 14);
@@ -133,47 +133,47 @@ add_task(function test_schema_version() 
 
     version = yield db.getSchemaVersion();
     do_check_eq(version, 14);
   }
 
   yield db.close();
 });
 
-add_task(function test_simple_insert() {
+add_task(function* test_simple_insert() {
   let c = yield getDummyDatabase("simple_insert");
 
   let result = yield c.execute("INSERT INTO dirs VALUES (NULL, 'foo')");
   do_check_true(Array.isArray(result));
   do_check_eq(result.length, 0);
   yield c.close();
 });
 
-add_task(function test_simple_bound_array() {
+add_task(function* test_simple_bound_array() {
   let c = yield getDummyDatabase("simple_bound_array");
 
   let result = yield c.execute("INSERT INTO dirs VALUES (?, ?)", [1, "foo"]);
   do_check_eq(result.length, 0);
   yield c.close();
 });
 
-add_task(function test_simple_bound_object() {
+add_task(function* test_simple_bound_object() {
   let c = yield getDummyDatabase("simple_bound_object");
   let result = yield c.execute("INSERT INTO dirs VALUES (:id, :path)",
                                {id: 1, path: "foo"});
   do_check_eq(result.length, 0);
   result = yield c.execute("SELECT id, path FROM dirs");
   do_check_eq(result.length, 1);
   do_check_eq(result[0].getResultByName("id"), 1);
   do_check_eq(result[0].getResultByName("path"), "foo");
   yield c.close();
 });
 
 // This is mostly a sanity test to ensure simple executions work.
-add_task(function test_simple_insert_then_select() {
+add_task(function* test_simple_insert_then_select() {
   let c = yield getDummyDatabase("simple_insert_then_select");
 
   yield c.execute("INSERT INTO dirs VALUES (NULL, 'foo')");
   yield c.execute("INSERT INTO dirs (path) VALUES (?)", ["bar"]);
 
   let result = yield c.execute("SELECT * FROM dirs");
   do_check_eq(result.length, 2);
 
@@ -186,82 +186,82 @@ add_task(function test_simple_insert_the
 
     let expected = {1: "foo", 2: "bar"}[i];
     do_check_eq(row.getResultByName("path"), expected);
   }
 
   yield c.close();
 });
 
-add_task(function test_repeat_execution() {
+add_task(function* test_repeat_execution() {
   let c = yield getDummyDatabase("repeat_execution");
 
   let sql = "INSERT INTO dirs (path) VALUES (:path)";
   yield c.executeCached(sql, {path: "foo"});
   yield c.executeCached(sql);
 
   let result = yield c.execute("SELECT * FROM dirs");
 
   do_check_eq(result.length, 2);
 
   yield c.close();
 });
 
-add_task(function test_table_exists() {
+add_task(function* test_table_exists() {
   let c = yield getDummyDatabase("table_exists");
 
   do_check_false(yield c.tableExists("does_not_exist"));
   do_check_true(yield c.tableExists("dirs"));
   do_check_true(yield c.tableExists("files"));
 
   yield c.close();
 });
 
-add_task(function test_index_exists() {
+add_task(function* test_index_exists() {
   let c = yield getDummyDatabase("index_exists");
 
   do_check_false(yield c.indexExists("does_not_exist"));
 
   yield c.execute("CREATE INDEX my_index ON dirs (path)");
   do_check_true(yield c.indexExists("my_index"));
 
   yield c.close();
 });
 
-add_task(function test_temp_table_exists() {
+add_task(function* test_temp_table_exists() {
   let c = yield getDummyTempDatabase("temp_table_exists");
 
   do_check_false(yield c.tableExists("temp_does_not_exist"));
   do_check_true(yield c.tableExists("dirs"));
   do_check_true(yield c.tableExists("files"));
 
   yield c.close();
 });
 
-add_task(function test_temp_index_exists() {
+add_task(function* test_temp_index_exists() {
   let c = yield getDummyTempDatabase("temp_index_exists");
 
   do_check_false(yield c.indexExists("temp_does_not_exist"));
 
   yield c.execute("CREATE INDEX my_index ON dirs (path)");
   do_check_true(yield c.indexExists("my_index"));
 
   yield c.close();
 });
 
-add_task(function test_close_cached() {
+add_task(function* test_close_cached() {
   let c = yield getDummyDatabase("close_cached");
 
   yield c.executeCached("SELECT * FROM dirs");
   yield c.executeCached("SELECT * FROM files");
 
   yield c.close();
 });
 
-add_task(function test_execute_invalid_statement() {
+add_task(function* test_execute_invalid_statement() {
   let c = yield getDummyDatabase("invalid_statement");
 
   let deferred = Promise.defer();
 
   do_check_eq(c._connectionData._anonymousStatements.size, 0);
 
   c.execute("SELECT invalid FROM unknown").then(do_throw, function onError(error) {
     deferred.resolve();
@@ -270,17 +270,17 @@ add_task(function test_execute_invalid_s
   yield deferred.promise;
 
   // Ensure we don't leak the statement instance.
   do_check_eq(c._connectionData._anonymousStatements.size, 0);
 
   yield c.close();
 });
 
-add_task(function test_on_row_exception_ignored() {
+add_task(function* test_on_row_exception_ignored() {
   let c = yield getDummyDatabase("on_row_exception_ignored");
 
   let sql = "INSERT INTO dirs (path) VALUES (?)";
   for (let i = 0; i < 10; i++) {
     yield c.executeCached(sql, ["dir" + i]);
   }
 
   let i = 0;
@@ -292,17 +292,17 @@ add_task(function test_on_row_exception_
 
   do_check_eq(hasResult, true);
   do_check_eq(i, 10);
 
   yield c.close();
 });
 
 // Ensure StopIteration during onRow causes processing to stop.
-add_task(function test_on_row_stop_iteration() {
+add_task(function* test_on_row_stop_iteration() {
   let c = yield getDummyDatabase("on_row_stop_iteration");
 
   let sql = "INSERT INTO dirs (path) VALUES (?)";
   for (let i = 0; i < 10; i++) {
     yield c.executeCached(sql, ["dir" + i]);
   }
 
   let i = 0;
@@ -316,72 +316,66 @@ add_task(function test_on_row_stop_itera
 
   do_check_eq(hasResult, true);
   do_check_eq(i, 5);
 
   yield c.close();
 });
 
 // Ensure execute resolves to false when no rows are selected.
-add_task(function test_on_row_stop_iteration() {
+add_task(function* test_on_row_stop_iteration() {
   let c = yield getDummyDatabase("no_on_row");
 
   let i = 0;
   let hasResult = yield c.execute(`SELECT * FROM dirs WHERE path="nonexistent"`, null, function onRow(row) {
     i++;
   });
 
   do_check_eq(hasResult, false);
   do_check_eq(i, 0);
 
   yield c.close();
 });
 
-add_task(function test_invalid_transaction_type() {
+add_task(function* test_invalid_transaction_type() {
   let c = yield getDummyDatabase("invalid_transaction_type");
 
-  let errored = false;
-  try {
-    c.executeTransaction(function () {}, "foobar");
-  } catch (ex) {
-    errored = true;
-    do_check_true(ex.message.startsWith("Unknown transaction type"));
-  } finally {
-    do_check_true(errored);
-  }
+  Assert.throws(() => c.executeTransaction(function* () {}, "foobar"),
+                /Unknown transaction type/,
+                "Unknown transaction type should throw");
 
   yield c.close();
 });
 
-add_task(function test_execute_transaction_success() {
+add_task(function* test_execute_transaction_success() {
   let c = yield getDummyDatabase("execute_transaction_success");
 
   do_check_false(c.transactionInProgress);
 
-  yield c.executeTransaction(function transaction(conn) {
+  yield c.executeTransaction(function* transaction(conn) {
     do_check_eq(c, conn);
     do_check_true(conn.transactionInProgress);
 
     yield conn.execute("INSERT INTO dirs (path) VALUES ('foo')");
   });
 
   do_check_false(c.transactionInProgress);
   let rows = yield c.execute("SELECT * FROM dirs");
   do_check_true(Array.isArray(rows));
   do_check_eq(rows.length, 1);
 
   yield c.close();
 });
 
-add_task(function test_execute_transaction_rollback() {
+add_task(function* test_execute_transaction_rollback() {
   let c = yield getDummyDatabase("execute_transaction_rollback");
 
   let deferred = Promise.defer();
 
-  c.executeTransaction(function transaction(conn) {
+  c.executeTransaction(function* transaction(conn) {
     yield conn.execute("INSERT INTO dirs (path) VALUES ('foo')");
     print("Expecting error with next statement.");
     yield conn.execute("INSERT INTO invalid VALUES ('foo')");
 
     // We should never get here.
     do_throw();
   }).then(do_throw, function onError(error) {
     deferred.resolve();
@@ -390,77 +384,107 @@ add_task(function test_execute_transacti
   yield deferred.promise;
 
   let rows = yield c.execute("SELECT * FROM dirs");
   do_check_eq(rows.length, 0);
 
   yield c.close();
 });
 
-add_task(function test_close_during_transaction() {
+add_task(function* test_close_during_transaction() {
   let c = yield getDummyDatabase("close_during_transaction");
 
   yield c.execute("INSERT INTO dirs (path) VALUES ('foo')");
 
-  let errored = false;
-  try {
-    yield c.executeTransaction(function transaction(conn) {
-      yield c.execute("INSERT INTO dirs (path) VALUES ('bar')");
-      yield c.close();
-    });
-  } catch (ex) {
-    errored = true;
-    do_check_eq(ex.message, "Connection being closed.");
-  } finally {
-    do_check_true(errored);
-  }
+  let promise = c.executeTransaction(function* transaction(conn) {
+    yield c.execute("INSERT INTO dirs (path) VALUES ('bar')");
+  });
+  yield c.close();
+
+  yield Assert.rejects(promise,
+                       /Transaction canceled due to a closed connection/,
+                       "closing a connection in the middle of a transaction should reject it");
 
   let c2 = yield getConnection("close_during_transaction");
   let rows = yield c2.execute("SELECT * FROM dirs");
   do_check_eq(rows.length, 1);
 
   yield c2.close();
 });
 
-add_task(function test_detect_multiple_transactions() {
+// Verify that we support concurrent transactions.
+add_task(function* test_multiple_transactions() {
   let c = yield getDummyDatabase("detect_multiple_transactions");
 
-  yield c.executeTransaction(function main() {
-    yield c.execute("INSERT INTO dirs (path) VALUES ('foo')");
-
-    let errored = false;
-    try {
-      yield c.executeTransaction(function child() {
-        yield c.execute("INSERT INTO dirs (path) VALUES ('bar')");
-      });
-    } catch (ex) {
-      errored = true;
-      do_check_true(ex.message.startsWith("A transaction is already active."));
-    } finally {
-      do_check_true(errored);
-    }
-  });
+  for (let i = 0; i < 10; ++i) {
+    // We don't wait for these transactions.
+    c.executeTransaction(function* () {
+      yield c.execute("INSERT INTO dirs (path) VALUES (:path)",
+                      { path: `foo${i}` });
+      yield c.execute("SELECT * FROM dirs");
+    });
+  }
+  for (let i = 0; i < 10; ++i) {
+    yield c.executeTransaction(function* () {
+      yield c.execute("INSERT INTO dirs (path) VALUES (:path)",
+                      { path: `bar${i}` });
+      yield c.execute("SELECT * FROM dirs");
+    });
+  }
 
   let rows = yield c.execute("SELECT * FROM dirs");
-  do_check_eq(rows.length, 1);
+  do_check_eq(rows.length, 20);
 
   yield c.close();
 });
 
-add_task(function test_shrink_memory() {
+// Verify that wrapped transactions ignore a BEGIN TRANSACTION failure, when
+// an externally opened transaction exists.
+add_task(function* test_wrapped_connection_transaction() {
+  let file = new FileUtils.File(OS.Path.join(OS.Constants.Path.profileDir,
+                                             "test_wrapStorageConnection.sqlite"));
+  let c = yield new Promise((resolve, reject) => {
+    Services.storage.openAsyncDatabase(file, null, (status, db) => {
+      if (Components.isSuccessCode(status)) {
+        resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
+      } else {
+        reject(new Error(status));
+      }
+    });
+  });
+
+  let wrapper = yield Sqlite.wrapStorageConnection({ connection: c });
+  // Start a transaction on the raw connection.
+  yield c.executeSimpleSQLAsync("BEGIN");
+  // Now use executeTransaction, it will be executed, but not in a transaction.
+  yield wrapper.executeTransaction(function* () {
+    yield wrapper.execute("CREATE TABLE test (id INTEGER PRIMARY KEY AUTOINCREMENT)");
+  });
+  // This should not fail cause the internal transaction has not been created.
+  yield c.executeSimpleSQLAsync("COMMIT");
+
+  yield wrapper.execute("SELECT * FROM test");
+
+  // Closing the wrapper should just finalize statements but not close the
+  // database.
+  yield wrapper.close();
+  yield c.asyncClose();
+});
+
+add_task(function* test_shrink_memory() {
   let c = yield getDummyDatabase("shrink_memory");
 
   // It's just a simple sanity test. We have no way of measuring whether this
   // actually does anything.
 
   yield c.shrinkMemory();
   yield c.close();
 });
 
-add_task(function test_no_shrink_on_init() {
+add_task(function* test_no_shrink_on_init() {
   let c = yield getConnection("no_shrink_on_init",
                               {shrinkMemoryOnConnectionIdleMS: 200});
 
   let oldShrink = c._connectionData.shrinkMemory;
   let count = 0;
   Object.defineProperty(c._connectionData, "shrinkMemory", {
     value: function () {
       count++;
@@ -473,17 +497,17 @@ add_task(function test_no_shrink_on_init
 
   yield c.execute("SELECT 1");
   yield sleep(220);
   do_check_eq(count, 1);
 
   yield c.close();
 });
 
-add_task(function test_idle_shrink_fires() {
+add_task(function* test_idle_shrink_fires() {
   let c = yield getDummyDatabase("idle_shrink_fires",
                                  {shrinkMemoryOnConnectionIdleMS: 200});
   c._connectionData._clearIdleShrinkTimer();
 
   let oldShrink = c._connectionData.shrinkMemory;
   let shrinkPromises = [];
 
   let count = 0;
@@ -515,17 +539,17 @@ add_task(function test_idle_shrink_fires
 
   do_check_eq(count, 2);
   do_check_eq(shrinkPromises.length, 1);
   yield shrinkPromises[0];
 
   yield c.close();
 });
 
-add_task(function test_idle_shrink_reset_on_operation() {
+add_task(function* test_idle_shrink_reset_on_operation() {
   const INTERVAL = 500;
   let c = yield getDummyDatabase("idle_shrink_reset_on_operation",
                                  {shrinkMemoryOnConnectionIdleMS: INTERVAL});
 
   c._connectionData._clearIdleShrinkTimer();
 
   let oldShrink = c._connectionData.shrinkMemory;
   let shrinkPromises = [];
@@ -563,17 +587,17 @@ add_task(function test_idle_shrink_reset
   // Ensure we fired.
   do_check_eq(count, 1);
   do_check_eq(shrinkPromises.length, 1);
   yield shrinkPromises[0];
 
   yield c.close();
 });
 
-add_task(function test_in_progress_counts() {
+add_task(function* test_in_progress_counts() {
   let c = yield getDummyDatabase("in_progress_counts");
   do_check_eq(c._connectionData._statementCounter, c._initialStatementCount);
   do_check_eq(c._connectionData._pendingStatements.size, 0);
   yield c.executeCached("INSERT INTO dirs (path) VALUES ('foo')");
   do_check_eq(c._connectionData._statementCounter, c._initialStatementCount + 1);
   do_check_eq(c._connectionData._pendingStatements.size, 0);
 
   let expectOne;
@@ -616,17 +640,17 @@ add_task(function test_in_progress_count
   do_check_eq(expectOne, 1);
   do_check_eq(expectTwo, 2);
   do_check_eq(c._connectionData._statementCounter, c._initialStatementCount + 3);
   do_check_eq(c._connectionData._pendingStatements.size, 0);
 
   yield c.close();
 });
 
-add_task(function test_discard_while_active() {
+add_task(function* test_discard_while_active() {
   let c = yield getDummyDatabase("discard_while_active");
 
   yield c.executeCached("INSERT INTO dirs (path) VALUES ('foo')");
   yield c.executeCached("INSERT INTO dirs (path) VALUES ('bar')");
 
   let discarded = -1;
   let first = true;
   let sql = "SELECT * FROM dirs";
@@ -642,17 +666,17 @@ add_task(function test_discard_while_act
   do_check_eq(3, discarded);
 
   // And again is safe.
   do_check_eq(0, c.discardCachedStatements());
 
   yield c.close();
 });
 
-add_task(function test_discard_cached() {
+add_task(function* test_discard_cached() {
   let c = yield getDummyDatabase("discard_cached");
 
   yield c.executeCached("SELECT * from dirs");
   do_check_eq(1, c._connectionData._cachedStatements.size);
 
   yield c.executeCached("SELECT * from files");
   do_check_eq(2, c._connectionData._cachedStatements.size);
 
@@ -660,17 +684,17 @@ add_task(function test_discard_cached() 
   do_check_eq(2, c._connectionData._cachedStatements.size);
 
   c.discardCachedStatements();
   do_check_eq(0, c._connectionData._cachedStatements.size);
 
   yield c.close();
 });
 
-add_task(function test_programmatic_binding() {
+add_task(function* test_programmatic_binding() {
   let c = yield getDummyDatabase("programmatic_binding");
 
   let bindings = [
     {id: 1,    path: "foobar"},
     {id: null, path: "baznoo"},
     {id: 5,    path: "toofoo"},
   ];
 
@@ -678,56 +702,56 @@ add_task(function test_programmatic_bind
   let result = yield c.execute(sql, bindings);
   do_check_eq(result.length, 0);
 
   let rows = yield c.executeCached("SELECT * from dirs");
   do_check_eq(rows.length, 3);
   yield c.close();
 });
 
-add_task(function test_programmatic_binding_transaction() {
+add_task(function* test_programmatic_binding_transaction() {
   let c = yield getDummyDatabase("programmatic_binding_transaction");
 
   let bindings = [
     {id: 1,    path: "foobar"},
     {id: null, path: "baznoo"},
     {id: 5,    path: "toofoo"},
   ];
 
   let sql = "INSERT INTO dirs VALUES (:id, :path)";
-  yield c.executeTransaction(function transaction() {
+  yield c.executeTransaction(function* transaction() {
     let result = yield c.execute(sql, bindings);
     do_check_eq(result.length, 0);
 
     let rows = yield c.executeCached("SELECT * from dirs");
     do_check_eq(rows.length, 3);
   });
 
   // Transaction committed.
   let rows = yield c.executeCached("SELECT * from dirs");
   do_check_eq(rows.length, 3);
   yield c.close();
 });
 
-add_task(function test_programmatic_binding_transaction_partial_rollback() {
+add_task(function* test_programmatic_binding_transaction_partial_rollback() {
   let c = yield getDummyDatabase("programmatic_binding_transaction_partial_rollback");
 
   let bindings = [
     {id: 2, path: "foobar"},
     {id: 3, path: "toofoo"},
   ];
 
   let sql = "INSERT INTO dirs VALUES (:id, :path)";
 
   // Add some data in an implicit transaction before beginning the batch insert.
   yield c.execute(sql, {id: 1, path: "works"});
 
   let secondSucceeded = false;
   try {
-    yield c.executeTransaction(function transaction() {
+    yield c.executeTransaction(function* transaction() {
       // Insert one row. This won't implicitly start a transaction.
       let result = yield c.execute(sql, bindings[0]);
 
       // Insert multiple rows. mozStorage will want to start a transaction.
       // One of the inserts will fail, so the transaction should be rolled back.
       result = yield c.execute(sql, bindings);
       secondSucceeded = true;
     });
@@ -741,21 +765,19 @@ add_task(function test_programmatic_bind
   // Everything that happened in *our* transaction, not mozStorage's, got
   // rolled back, but the first row still exists.
   let rows = yield c.executeCached("SELECT * from dirs");
   do_check_eq(rows.length, 1);
   do_check_eq(rows[0].getResultByName("path"), "works");
   yield c.close();
 });
 
-/**
- * Just like the previous test, but relying on the implicit
- * transaction established by mozStorage.
- */
-add_task(function test_programmatic_binding_implicit_transaction() {
+// Just like the previous test, but relying on the implicit
+// transaction established by mozStorage.
+add_task(function* test_programmatic_binding_implicit_transaction() {
   let c = yield getDummyDatabase("programmatic_binding_implicit_transaction");
 
   let bindings = [
     {id: 2, path: "foobar"},
     {id: 1, path: "toofoo"},
   ];
 
   let sql = "INSERT INTO dirs VALUES (:id, :path)";
@@ -772,21 +794,19 @@ add_task(function test_programmatic_bind
 
   // The entire batch failed.
   let rows = yield c.executeCached("SELECT * from dirs");
   do_check_eq(rows.length, 1);
   do_check_eq(rows[0].getResultByName("path"), "works");
   yield c.close();
 });
 
-/**
- * Test that direct binding of params and execution through mozStorage doesn't
- * error when we manually create a transaction. See Bug 856925.
- */
-add_task(function test_direct() {
+// Test that direct binding of params and execution through mozStorage doesn't
+// error when we manually create a transaction. See Bug 856925.
+add_task(function* test_direct() {
   let file = FileUtils.getFile("TmpD", ["test_direct.sqlite"]);
   file.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, FileUtils.PERMS_FILE);
   print("Opening " + file.path);
 
   let db = Services.storage.openDatabase(file);
   print("Opened " + db);
 
   db.executeSimpleSQL("CREATE TABLE types (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, UNIQUE (name))");
@@ -851,19 +871,17 @@ add_task(function test_direct() {
 
   deferred = Promise.defer();
   db.asyncClose(function () {
     deferred.resolve()
   });
   yield deferred.promise;
 });
 
-/**
- * Test Sqlite.cloneStorageConnection.
- */
+// Test Sqlite.cloneStorageConnection.
 add_task(function* test_cloneStorageConnection() {
   let file = new FileUtils.File(OS.Path.join(OS.Constants.Path.profileDir,
                                              "test_cloneStorageConnection.sqlite"));
   let c = yield new Promise((resolve, reject) => {
     Services.storage.openAsyncDatabase(file, null, (status, db) => {
       if (Components.isSuccessCode(status)) {
         resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
@@ -881,43 +899,37 @@ add_task(function* test_cloneStorageConn
   yield clone2.execute("CREATE TABLE test (id INTEGER PRIMARY KEY)");
 
   // Closing order should not matter.
   yield c.asyncClose();
   yield clone2.close();
   yield clone.close();
 });
 
-/**
- * Test Sqlite.cloneStorageConnection invalid argument.
- */
+// Test Sqlite.cloneStorageConnection invalid argument.
 add_task(function* test_cloneStorageConnection() {
   try {
     let clone = yield Sqlite.cloneStorageConnection({ connection: null });
     do_throw(new Error("Should throw on invalid connection"));
   } catch (ex if ex.name == "TypeError") {}
 });
 
-/**
- * Test clone() method.
- */
+// Test clone() method.
 add_task(function* test_clone() {
   let c = yield getDummyDatabase("clone");
 
   let clone = yield c.clone();
   // Just check that it works.
   yield clone.execute("SELECT 1");
   // Closing order should not matter.
   yield c.close();
   yield clone.close();
 });
 
-/**
- * Test clone(readOnly) method.
- */
+// Test clone(readOnly) method.
 add_task(function* test_readOnly_clone() {
   let path = OS.Path.join(OS.Constants.Path.profileDir, "test_readOnly_clone.sqlite");
   let c = yield Sqlite.openConnection({path: path, sharedMemoryCache: false});
 
   let clone = yield c.clone(true);
   // Just check that it works.
   yield clone.execute("SELECT 1");
   // But should not be able to write.
@@ -925,19 +937,17 @@ add_task(function* test_readOnly_clone()
     yield clone.execute("CREATE TABLE test (id INTEGER PRIMARY KEY)");
     do_throw(new Error("Should not be able to write to a read-only clone."));
   } catch (ex) {}
   // Closing order should not matter.
   yield c.close();
   yield clone.close();
 });
 
-/**
- * Test Sqlite.wrapStorageConnection.
- */
+// Test Sqlite.wrapStorageConnection.
 add_task(function* test_wrapStorageConnection() {
   let file = new FileUtils.File(OS.Path.join(OS.Constants.Path.profileDir,
                                              "test_wrapStorageConnection.sqlite"));
   let c = yield new Promise((resolve, reject) => {
     Services.storage.openAsyncDatabase(file, null, (status, db) => {
       if (Components.isSuccessCode(status)) {
         resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
@@ -952,19 +962,17 @@ add_task(function* test_wrapStorageConne
   yield wrapper.executeCached("SELECT 1");
 
   // Closing the wrapper should just finalize statements but not close the
   // database.
   yield wrapper.close();
   yield c.asyncClose();
 });
 
-/**
- * Test finalization
- */
+// Test finalization
 add_task(function* test_closed_by_witness() {
   failTestsOnAutoClose(false);
   let c = yield getDummyDatabase("closed_by_witness");
 
   Services.obs.notifyObservers(null, "sqlite-finalization-witness",
                                c._connectionData._identifier);
   // Since we triggered finalization ourselves, tell the witness to
   // forget the connection so it does not trigger a finalization again
@@ -1059,11 +1067,14 @@ add_task(function* test_close_database_o
   // references. This is needed at the moment, otherwise
   // garbage-collection takes place after the shutdown barrier and the
   // test will timeout. Once that is fixed, we can remove this line
   // and be fine as long as the connections are garbage-collected.
   let last = yield getDummyDatabase("gc_last");
   yield last.close();
 
   Components.utils.forceGC();
+  Components.utils.forceCC();
+  Components.utils.forceShrinkingGC();
+
   yield finalPromise;
   failTestsOnAutoClose(true);
 });