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 347642 ef6bdef5db2c56f9b511393d273fecdbcd18ba97
parent 347641 36b1cce63e74864b9e9df1522d6a9955eb011ca2
child 347643 4e8e4c0bfd0b61a668911d1261b51b5083bd529e
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1310152
milestone52.0a1
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);