Bug 1310152 - AsyncClone may try to reuse a freed Critical Section in Sqlite. r=asuth
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 03 Nov 2016 15:31:31 +0100
changeset 363800 ef6bdef5db2c56f9b511393d273fecdbcd18ba97
parent 363799 36b1cce63e74864b9e9df1522d6a9955eb011ca2
child 363801 4e8e4c0bfd0b61a668911d1261b51b5083bd529e
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1310152
milestone52.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 1310152 - AsyncClone may try to reuse a freed Critical Section in Sqlite. r=asuth MozReview-Commit-ID: LW0esWlNfQQ
storage/mozStorageConnection.cpp
storage/test/unit/test_storage_connection.js
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -421,17 +421,17 @@ public:
     , mClone(aClone)
     , mReadOnly(aReadOnly)
     , mCallback(aCallback)
   {
     MOZ_ASSERT(NS_IsMainThread());
   }
 
   NS_IMETHOD Run() override {
-    MOZ_ASSERT (NS_GetCurrentThread() == mClone->getAsyncExecutionTarget());
+    MOZ_ASSERT (NS_GetCurrentThread() == mConnection->getAsyncExecutionTarget());
 
     nsresult rv = mConnection->initializeClone(mClone, mReadOnly);
     if (NS_FAILED(rv)) {
       return Dispatch(rv, nullptr);
     }
     return Dispatch(NS_OK,
                     NS_ISUPPORTS_CAST(mozIStorageAsyncConnection*, mClone));
   }
@@ -1327,17 +1327,20 @@ Connection::AsyncClone(bool aReadOnly,
     flags = (~SQLITE_OPEN_CREATE & flags);
   }
 
   RefPtr<Connection> clone = new Connection(mStorageService, flags,
                                               mAsyncOnly);
 
   RefPtr<AsyncInitializeClone> initEvent =
     new AsyncInitializeClone(this, clone, aReadOnly, aCallback);
-  nsCOMPtr<nsIEventTarget> target = clone->getAsyncExecutionTarget();
+  // Dispatch to our async thread, since the originating connection must remain
+  // valid and open for the whole cloning process.  This also ensures we are
+  // properly serialized with a `close` operation, rather than race with it.
+  nsCOMPtr<nsIEventTarget> target = getAsyncExecutionTarget();
   if (!target) {
     return NS_ERROR_UNEXPECTED;
   }
   return target->Dispatch(initEvent, NS_DISPATCH_NORMAL);
 }
 
 nsresult
 Connection::initializeClone(Connection* aClone, bool aReadOnly)
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -426,25 +426,26 @@ add_task(function* test_async_open_with_
     }
   });
   do_check_true(found);
   stmt.finalize();
   yield asyncClose(adb);
 });
 
 add_task(function* test_clone_trivial_async() {
-  let db1 = getService().openDatabase(getTestDB());
-  do_print("Opened adb1");
-  do_check_true(db1 instanceof Ci.mozIStorageAsyncConnection);
-  let adb2 = yield asyncClone(db1, true);
-  do_check_true(adb2 instanceof Ci.mozIStorageAsyncConnection);
-  do_print("Cloned to adb2");
-  db1.close();
-  do_print("Closed db1");
-  yield asyncClose(adb2);
+  do_print("Open connection");
+  let db = getService().openDatabase(getTestDB());
+  do_check_true(db instanceof Ci.mozIStorageAsyncConnection);
+  do_print("AsyncClone connection");
+  let clone = yield asyncClone(db, true);
+  do_check_true(clone instanceof Ci.mozIStorageAsyncConnection);
+  do_print("Close connection");
+  yield asyncClose(db);
+  do_print("Close clone");
+  yield asyncClose(clone);
 });
 
 add_task(function* test_clone_no_optional_param_async() {
   "use strict";
   do_print("Testing async cloning");
   let adb1 = yield openAsyncDatabase(getTestDB(), null);
   do_check_true(adb1 instanceof Ci.mozIStorageAsyncConnection);