Bug 1068620 - Provide a Sqlite.wrapStorageConnection method. r=Yoric
authorMarco Bonardo <mbonardo@mozilla.com>
Mon, 22 Sep 2014 18:32:43 +0200
changeset 206490 fd762b4aa27e67272fc49e0c95194f80074915cb
parent 206489 3699909158469cc32c9f7c1376993618471b1c7c
child 206491 fac90451603fbf142fdafec8cebce4da986a1461
push id27528
push userryanvm@gmail.com
push dateMon, 22 Sep 2014 19:27:54 +0000
treeherdermozilla-central@d8688cafc752 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs1068620
milestone35.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 1068620 - Provide a Sqlite.wrapStorageConnection method. r=Yoric
toolkit/modules/Sqlite.jsm
toolkit/modules/tests/xpcshell/test_sqlite.js
--- a/toolkit/modules/Sqlite.jsm
+++ b/toolkit/modules/Sqlite.jsm
@@ -32,16 +32,22 @@ XPCOMUtils.defineLazyServiceGetter(this,
                                    "@mozilla.org/toolkit/finalizationwitness;1",
                                    "nsIFinalizationWitnessService");
 
 
 // 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
+// to differentiate their behavior.
+let wrappedConnections = new Set();
+
 /**
  * Once `true`, reject any attempt to open or close a database.
  */
 let isClosed = false;
 
 let Debugging = {
   // Tests should fail if a connection auto closes.  The exception is
   // when finalization itself is tested, in which case this flag
@@ -62,16 +68,30 @@ function logScriptError(message) {
   // flag can be used to suppress this for tests that explicitly
   // test auto closes.
   if (Debugging.failTestsOnAutoClose) {
     Promise.reject(new Error(message));
   }
 }
 
 /**
+ * Gets connection identifier from its database file path.
+ *
+ * @param path
+ *        A file string path pointing to a database file.
+ * @return the connection identifier.
+ */
+function getIdentifierByPath(path) {
+  let basename = OS.Path.basename(path);
+  let number = connectionCounters.get(basename) || 0;
+  connectionCounters.set(basename, number + 1);
+  return basename + "#" + number;
+}
+
+/**
  * Barriers used to ensure that Sqlite.jsm is shutdown after all
  * its clients.
  */
 XPCOMUtils.defineLazyGetter(this, "Barriers", () => {
   let Barriers = {
     /**
      * Public barrier that clients may use to add blockers to the
      * shutdown of Sqlite.jsm. Triggered by profile-before-change.
@@ -90,27 +110,27 @@ XPCOMUtils.defineLazyGetter(this, "Barri
 
   /**
    * Observer for the event which is broadcasted when the finalization
    * witness `_witness` of `OpenedConnection` is garbage collected.
    *
    * The observer is passed the connection identifier of the database
    * connection that is being finalized.
    */
-  let finalizationObserver = function (subject, topic, connectionIdentifier) {
-    let connectionData = ConnectionData.byId.get(connectionIdentifier);
+  let finalizationObserver = function (subject, topic, identifier) {
+    let connectionData = ConnectionData.byId.get(identifier);
 
     if (connectionData === undefined) {
       logScriptError("Error: Attempt to finalize unknown Sqlite connection: " +
-                     connectionIdentifier + "\n");
+                     identifier + "\n");
       return;
     }
 
-    ConnectionData.byId.delete(connectionIdentifier);
-    logScriptError("Warning: Sqlite connection '" + connectionIdentifier +
+    ConnectionData.byId.delete(identifier);
+    logScriptError("Warning: Sqlite connection '" + identifier +
                    "' was not properly closed. Auto-close triggered by garbage collection.\n");
     connectionData.close();
   };
   Services.obs.addObserver(finalizationObserver, "sqlite-finalization-witness", false);
 
   /**
    * Ensure that Sqlite.jsm:
    * - informs its clients before shutting down;
@@ -165,23 +185,27 @@ XPCOMUtils.defineLazyGetter(this, "Barri
  * a garbage collection of a finalization witness in
  * 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, basename, number, options) {
-  this._log = Log.repository.getLoggerWithMessagePrefix("Sqlite.Connection." + basename,
-                                                        "Conn #" + number + ": ");
+function ConnectionData(connection, identifier, options={}) {
+  this._log = Log.repository.getLoggerWithMessagePrefix("Sqlite.Connection." +
+                                                        identifier + ": ");
   this._log.info("Opened");
 
   this._dbConn = connection;
-  this._connectionIdentifier = basename + " Conn #" + number;
+
+  // 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;
+
   this._open = true;
 
   this._cachedStatements = new Map();
   this._anonymousStatements = new Map();
   this._anonymousCounter = 0;
 
   // A map from statement index to mozIStoragePendingStatement, to allow for
   // canceling prior to finalizing the mozIStorageStatements.
@@ -199,37 +223,37 @@ function ConnectionData(connection, base
     // We wait for the first statement execute to start the timer because
     // shrinking now would not do anything.
   }
 
   this._deferredClose = Promise.defer();
   this._closeRequested = false;
 
   Barriers.connections.client.addBlocker(
-    this._connectionIdentifier + ": waiting for shutdown",
+    this._identifier + ": waiting for shutdown",
     this._deferredClose.promise,
     () =>  ({
-      identifier: this._connectionIdentifier,
+      identifier: this._identifier,
       isCloseRequested: this._closeRequested,
       hasDbConn: !!this._dbConn,
       hasInProgressTransaction: !!this._inProgressTransaction,
       pendingStatements: this._pendingStatements.size,
       statementCounter: this._statementCounter,
     })
   );
 }
 
 /**
  * Map of connection identifiers to ConnectionData objects
  *
  * The connection identifier is a human-readable name of the
  * database. Used by finalization witnesses to be able to close opened
  * connections on garbage collection.
  *
- * Key: _connectionIdentifier of ConnectionData
+ * Key: _identifier of ConnectionData
  * Value: ConnectionData object
  */
 ConnectionData.byId = new Map();
 
 ConnectionData.prototype = Object.freeze({
   close: function () {
     this._closeRequested = true;
 
@@ -299,25 +323,33 @@ ConnectionData.prototype = Object.freeze
       statement.finalize();
     }
     this._cachedStatements.clear();
 
     // This guards against operations performed between the call to this
     // function and asyncClose() finishing. See also bug 726990.
     this._open = false;
 
-    this._log.debug("Calling asyncClose().");
-    this._dbConn.asyncClose(() => {
+    // 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();
-    });
+    }
+    if (wrappedConnections.has(this._identifier)) {
+      wrappedConnections.delete(this._identifier);
+      markAsClosed();
+    } else {
+      this._log.debug("Calling asyncClose().");
+      this._dbConn.asyncClose(markAsClosed);
+    }
   },
 
   executeCached: function (sql, params=null, onRow=null) {
     this.ensureOpen();
 
     if (!sql) {
       throw new Error("sql argument is empty.");
     }
@@ -717,22 +749,17 @@ function openConnection(options) {
                       "Got: " + options.shrinkMemoryOnConnectionIdleMS);
     }
 
     openedOptions.shrinkMemoryOnConnectionIdleMS =
       options.shrinkMemoryOnConnectionIdleMS;
   }
 
   let file = FileUtils.File(path);
-
-  let basename = OS.Path.basename(path);
-  let number = connectionCounters.get(basename) || 0;
-  connectionCounters.set(basename, number + 1);
-
-  let identifier = basename + "#" + number;
+  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);
@@ -741,30 +768,30 @@ function openConnection(options) {
     if (!connection) {
       log.warn("Could not open connection: " + status);
       deferred.reject(new Error("Could not open connection: " + status));
       return;
     }
     log.info("Connection opened");
     try {
       deferred.resolve(
-        new OpenedConnection(connection.QueryInterface(Ci.mozIStorageAsyncConnection), basename, number,
-        openedOptions));
+        new OpenedConnection(connection.QueryInterface(Ci.mozIStorageAsyncConnection),
+                            identifier, openedOptions));
     } catch (ex) {
       log.warn("Could not open database: " + CommonUtils.exceptionStr(ex));
       deferred.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 on OpenedConnection handle.
+ * returned in form of an OpenedConnection handle.
  *
  * The following parameters can control the cloned connection:
  *
  *   connection -- (mozIStorageAsyncConnection) The original Storage connection
  *       to clone.  It's not possible to clone connections to memory databases.
  *
  *   readOnly -- (boolean) - If true the clone will be read-only.  If the
  *       original connection is already read-only, the clone will be, regardless
@@ -807,43 +834,88 @@ function cloneStorageConnection(options)
       throw new TypeError("shrinkMemoryOnConnectionIdleMS must be an integer. " +
                           "Got: " + options.shrinkMemoryOnConnectionIdleMS);
     }
     openedOptions.shrinkMemoryOnConnectionIdleMS =
       options.shrinkMemoryOnConnectionIdleMS;
   }
 
   let path = source.databaseFile.path;
-  let basename = OS.Path.basename(path);
-  let number = connectionCounters.get(basename) || 0;
-  connectionCounters.set(basename, number + 1);
-  let identifier = basename + "#" + number;
+  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, basename, number,
-                                            openedOptions));
+      deferred.resolve(new OpenedConnection(conn, identifier, openedOptions));
     } catch (ex) {
       log.warn("Could not clone database: " + CommonUtils.exceptionStr(ex));
       deferred.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
+ * underlying mozStorage connection.
+ *
+ * The following parameters can control the wrapped connection:
+ *
+ *   connection -- (mozIStorageAsyncConnection) The original Storage connection
+ *       to wrap.
+ *
+ * @param options
+ *        (Object) Parameters to control connection and wrap options.
+ *
+ * @return Promise<OpenedConnection>
+ */
+function wrapStorageConnection(options) {
+  let log = Log.repository.getLogger("Sqlite.ConnectionWrapper");
+
+  let connection = options && options.connection;
+  if (!connection || !(connection instanceof Ci.mozIStorageAsyncConnection)) {
+    throw new TypeError("connection not specified or invalid.");
+  }
+
+  if (isClosed) {
+    throw new Error("Sqlite.jsm has been shutdown. Cannot wrap connection to: " + connection.database.path);
+  }
+
+  let path = connection.databaseFile.path;
+  let identifier = getIdentifierByPath(path);
+
+  log.info("Wrapping database: " + path + " (" + identifier + ")");
+  return new Promise(resolve => {
+    try {
+      let conn = connection.QueryInterface(Ci.mozIStorageAsyncConnection);
+      let wrapper = new OpenedConnection(conn, identifier);
+      // We must not handle shutdown of a wrapped connection, since that is
+      // already handled by the opener.
+      wrappedConnections.add(identifier);
+      resolve(wrapper);
+    } catch (ex) {
+      log.warn("Could not wrap database: " + CommonUtils.exceptionStr(ex));
+      throw ex;
+    }
+  });
+}
+
+/**
  * Handle on an opened SQLite database.
  *
  * This is essentially a glorified wrapper around mozIStorageConnection.
  * However, it offers some compelling advantages.
  *
  * The main functions on this type are `execute` and `executeCached`. These are
  * ultimately how all SQL statements are executed. It's worth explaining their
  * differences.
@@ -872,44 +944,43 @@ function cloneStorageConnection(options)
  *   Ability to enqueue operations. Currently there can be race conditions,
  *   especially as far as transactions are concerned. It would be nice to have
  *   an enqueueOperation(func) API that serially executes passed functions.
  *
  *   Support for SAVEPOINT (named/nested transactions) might be useful.
  *
  * @param connection
  *        (mozIStorageConnection) Underlying SQLite connection.
- * @param basename
- *        (string) The basename of this database name. Used for logging.
- * @param number
- *        (Number) The connection number to this database.
- * @param options
+ * @param identifier
+ *        (string) The unique identifier of this database. It may be used for
+ *        logging or as a key in Maps.
+ * @param options [optional]
  *        (object) Options to control behavior of connection. See
  *        `openConnection`.
  */
-function OpenedConnection(connection, basename, number, options) {
+function OpenedConnection(connection, identifier, options={}) {
   // Store all connection data in a field distinct from the
   // witness. This enables us to store an additional reference to this
   // field without preventing garbage collection of
   // OpenedConnection. On garbage collection, we will still be able to
   // close the database using this extra reference.
-  this._connectionData = new ConnectionData(connection, basename, number, options);
+  this._connectionData = new ConnectionData(connection, identifier, options);
 
   // Store the extra reference in a map with connection identifier as
   // key.
-  ConnectionData.byId.set(this._connectionData._connectionIdentifier,
+  ConnectionData.byId.set(this._connectionData._identifier,
                           this._connectionData);
 
   // Make a finalization witness. If this object is garbage collected
   // before its `forget` method has been called, an event with topic
   // "sqlite-finalization-witness" is broadcasted along with the
   // connection identifier string of the database.
   this._witness = FinalizationWitnessService.make(
     "sqlite-finalization-witness",
-    this._connectionData._connectionIdentifier);
+    this._connectionData._identifier);
 }
 
 OpenedConnection.prototype = Object.freeze({
   TRANSACTION_DEFERRED: "DEFERRED",
   TRANSACTION_IMMEDIATE: "IMMEDIATE",
   TRANSACTION_EXCLUSIVE: "EXCLUSIVE",
 
   TRANSACTION_TYPES: ["DEFERRED", "IMMEDIATE", "EXCLUSIVE"],
@@ -959,18 +1030,18 @@ OpenedConnection.prototype = Object.free
    * reopened.
    *
    * @return Promise<>
    */
   close: function () {
     // Unless cleanup has already been done by a previous call to
     // `close`, delete the database entry from map and tell the
     // finalization witness to forget.
-    if (ConnectionData.byId.has(this._connectionData._connectionIdentifier)) {
-      ConnectionData.byId.delete(this._connectionData._connectionIdentifier);
+    if (ConnectionData.byId.has(this._connectionData._identifier)) {
+      ConnectionData.byId.delete(this._connectionData._identifier);
       this._witness.forget();
     }
     return this._connectionData.close();
   },
 
   /**
    * Clones this connection to a new Sqlite one.
    *
@@ -1170,16 +1241,17 @@ OpenedConnection.prototype = Object.free
   discardCachedStatements: function () {
     return this._connectionData.discardCachedStatements();
   },
 });
 
 this.Sqlite = {
   openConnection: openConnection,
   cloneStorageConnection: cloneStorageConnection,
+  wrapStorageConnection: wrapStorageConnection,
   /**
    * Shutdown barrier client. May be used by clients to perform last-minute
    * cleanup prior to the shutdown of this module.
    *
    * See the documentation of AsyncShutdown.Barrier.prototype.client.
    */
   get shutdown() {
     return Barriers.shutdown.client;
--- a/toolkit/modules/tests/xpcshell/test_sqlite.js
+++ b/toolkit/modules/tests/xpcshell/test_sqlite.js
@@ -840,22 +840,22 @@ add_task(function test_direct() {
 });
 
 /**
  * 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((success, failure) => {
+  let c = yield new Promise((resolve, reject) => {
     Services.storage.openAsyncDatabase(file, null, (status, db) => {
       if (Components.isSuccessCode(status)) {
-        success(db.QueryInterface(Ci.mozIStorageAsyncConnection));
+        resolve(db.QueryInterface(Ci.mozIStorageAsyncConnection));
       } else {
-        failure(new Error(status));
+        reject(new Error(status));
       }
     });
   });
 
   let clone = yield Sqlite.cloneStorageConnection({ connection: c, readOnly: true });
   // Just check that it works.
   yield clone.execute("SELECT 1");
 
@@ -909,51 +909,78 @@ add_task(function* test_readOnly_clone()
     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.
+ */
+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 {
+        reject(new Error(status));
+      }
+    });
+  });
+
+  let wrapper = yield Sqlite.wrapStorageConnection({ connection: c });
+  // Just check that it works.
+  yield wrapper.execute("SELECT 1");
+  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
  */
 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._connectionIdentifier);
+                               c._connectionData._identifier);
   // Since we triggered finalization ourselves, tell the witness to
   // forget the connection so it does not trigger a finalization again
   c._witness.forget();
   yield c._connectionData._deferredClose.promise;
   do_check_false(c._connectionData._open);
   failTestsOnAutoClose(true);
 });
 
 add_task(function* test_warning_message_on_finalization() {
   failTestsOnAutoClose(false);
   let c = yield getDummyDatabase("warning_message_on_finalization");
-  let connectionIdentifier = c._connectionData._connectionIdentifier;
+  let identifier = c._connectionData._identifier;
   let deferred = Promise.defer();
 
   let listener = {
     observe: function(msg) {
       let messageText = msg.message;
       // Make sure the message starts with a warning containing the
       // connection identifier
-      if (messageText.indexOf("Warning: Sqlite connection '" + connectionIdentifier + "'") !== -1) {
+      if (messageText.indexOf("Warning: Sqlite connection '" + identifier + "'") !== -1) {
         deferred.resolve();
       }
     }
   };
   Services.console.registerListener(listener);
 
-  Services.obs.notifyObservers(null, "sqlite-finalization-witness", connectionIdentifier);
+  Services.obs.notifyObservers(null, "sqlite-finalization-witness", identifier);
   // Since we triggered finalization ourselves, tell the witness to
   // forget the connection so it does not trigger a finalization again
   c._witness.forget();
 
   yield deferred.promise;
   Services.console.unregisterListener(listener);
   failTestsOnAutoClose(true);
 });