Bug 945813 - Paper over cycle collection problem in IndexedDB. r=mccr8, r=khuey
☠☠ backed out by cb35692e00ec ☠ ☠
authorAndy Wingo <wingo@igalia.com>
Tue, 03 Dec 2013 18:35:04 +0100
changeset 158575 2572592c326dc2c632351877fa517f3f67c2ffcb
parent 158574 65a57bce96ef0fcaa914b4114e16e92afb667731
child 158576 4cbb184bf6cb68fb252af668d9fe4efdda5c374c
push id37032
push userryanvm@gmail.com
push dateTue, 03 Dec 2013 20:26:03 +0000
treeherdermozilla-inbound@4cbb184bf6cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8, khuey
bugs945813
milestone28.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 945813 - Paper over cycle collection problem in IndexedDB. r=mccr8, r=khuey
dom/indexedDB/test/unit/test_cursor_cycle.js
dom/indexedDB/test/unit/test_cursor_mutation.js
dom/indexedDB/test/unit/test_index_object_cursors.js
dom/indexedDB/test/unit/test_index_update_delete.js
dom/indexedDB/test/unit/xpcshell.ini
xpcom/base/nsCycleCollector.cpp
new file mode 100644
--- /dev/null
+++ b/dom/indexedDB/test/unit/test_cursor_cycle.js
@@ -0,0 +1,50 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+var testGenerator = testSteps();
+
+function testSteps()
+{
+  const Bob = { ss: "237-23-7732", name: "Bob" };
+
+  let request = indexedDB.open(this.window ? window.location.pathname : "Splendid Test", 1);
+  request.onerror = errorHandler;
+  request.onupgradeneeded = grabEventAndContinueHandler;
+  let event = yield undefined;
+
+  let db = event.target.result;
+  event.target.onsuccess = continueToNextStep;
+
+  let objectStore = db.createObjectStore("foo", { keyPath: "ss" });
+  objectStore.createIndex("name", "name", { unique: true });
+  objectStore.add(Bob);
+  yield undefined;
+
+  // This function expression causes objectStore to be aliased, and thus
+  // allocated on the scope chain.  Comment it out and the test passes.
+  // Bug 943409.
+  (function() { objectStore });
+
+  db.transaction("foo", "readwrite").objectStore("foo")
+    .index("name").openCursor().onsuccess = function(event) {
+    event.target.transaction.oncomplete = continueToNextStep;
+    let cursor = event.target.result;
+    if (cursor) {
+      let objectStore = event.target.transaction.objectStore("foo");
+      objectStore.delete(Bob.ss)
+                 .onsuccess = function(event) { cursor.continue(); };
+    }
+  };
+  yield undefined;
+  finishTest();
+
+  // This workaround allows this cycle test to pass, as the data
+  // associated with the transaction is collected, and thus there is no
+  // more cycle in which the outer objectStore participates.  Bug
+  // 943409.
+  gc();
+
+  yield undefined;
+}
--- a/dom/indexedDB/test/unit/test_cursor_mutation.js
+++ b/dom/indexedDB/test/unit/test_cursor_mutation.js
@@ -30,16 +30,21 @@ function testSteps()
   let event = yield undefined;
 
   let db = event.target.result;
   event.target.onsuccess = continueToNextStep;
 
   let objectStore = db.createObjectStore("foo", { keyPath: "ss" });
   objectStore.createIndex("name", "name", { unique: true });
 
+  // This function expression causes objectStore to be aliased, and thus
+  // allocated on the scope chain.  Comment it out and the test passes.
+  // Bug 943409.
+  (function() { objectStore });
+
   for (let i = 0; i < objectStoreData.length - 1; i++) {
     objectStore.add(objectStoreData[i]);
   }
   yield undefined;
 
   let count = 0;
 
   let sawAdded = false;
@@ -106,10 +111,17 @@ function testSteps()
     };
   yield undefined;
 
   is(count, objectStoreData.length - 1, "Good final count");
   is(sawAdded, true, "Saw item that was added");
   is(sawRemoved, false, "Didn't see item that was removed");
 
   finishTest();
+
+  // This workaround allows this cycle test to pass, as the data
+  // associated with the transaction is collected, and thus there is no
+  // more cycle in which the outer objectStore participates.  Bug
+  // 943409.
+  gc();
+
   yield undefined;
 }
--- a/dom/indexedDB/test/unit/test_index_object_cursors.js
+++ b/dom/indexedDB/test/unit/test_index_object_cursors.js
@@ -29,20 +29,27 @@ function testSteps()
   request.onupgradeneeded = grabEventAndContinueHandler;
   let event = yield undefined;
 
   let db = event.target.result;
   db.onerror = errorHandler;
 
   event.target.onsuccess = continueToNextStep;
 
+  // Bug 943409.
+  var bug943409 = [];
+  (function () { bug943409 })();
+
   for (let objectStoreIndex in objectStoreData) {
     const objectStoreInfo = objectStoreData[objectStoreIndex];
     let objectStore = db.createObjectStore(objectStoreInfo.name,
                                            objectStoreInfo.options);
+
+    bug943409.push(objectStore);
+
     for (let indexIndex in indexData) {
       const indexInfo = indexData[indexIndex];
       let index = objectStore.createIndex(indexInfo.name,
                                           indexInfo.keyPath,
                                           indexInfo.options);
     }
   }
   yield undefined;
@@ -136,10 +143,12 @@ function testSteps()
       db.transaction(objectStoreName, "readwrite")
         .objectStore(objectStoreName).clear()
         .onsuccess = continueToNextStep;
       yield undefined;
     }
   }
 
   finishTest();
+  // Bug 943409.
+  gc();
   yield undefined;
 }
--- a/dom/indexedDB/test/unit/test_index_update_delete.js
+++ b/dom/indexedDB/test/unit/test_index_update_delete.js
@@ -13,21 +13,28 @@ function testSteps()
   request.onupgradeneeded = grabEventAndContinueHandler;
   request.onsuccess = grabEventAndContinueHandler;
 
   let event = yield undefined;
 
   let db = event.target.result;
   db.onerror = errorHandler;
 
+  // Bug 943409.
+  var bug943409 = [];
+  (function () { bug943409 })();
+
   for each (let autoIncrement in [false, true]) {
     let objectStore =
       db.createObjectStore(autoIncrement, { keyPath: "id",
                                             autoIncrement: autoIncrement });
 
+    // Bug 943409.
+    bug943409.push(objectStore);
+
     for (let i = 0; i < 10; i++) {
       objectStore.add({ id: i, index: i });
     }
 
     for each (let unique in [false, true]) {
       objectStore.createIndex(unique, "index", { unique: unique });
     }
 
@@ -158,10 +165,12 @@ function testSteps()
       event = yield undefined;
 
       is(event.target.result, indexCount,
          "Correct number of entries in index");
     }
   }
 
   finishTest();
+  // Bug 943409.
+  gc();
   yield undefined;
 }
--- a/dom/indexedDB/test/unit/xpcshell.ini
+++ b/dom/indexedDB/test/unit/xpcshell.ini
@@ -16,16 +16,17 @@ support-files =
 [test_autoIncrement.js]
 [test_autoIncrement_indexes.js]
 [test_clear.js]
 [test_complex_keyPaths.js]
 [test_count.js]
 [test_create_index.js]
 [test_create_index_with_integer_keys.js]
 [test_create_objectStore.js]
+[test_cursor_cycle.js]
 [test_cursor_mutation.js]
 [test_cursor_update_updates_indexes.js]
 [test_cursors.js]
 [test_deleteDatabase.js]
 [test_deleteDatabase_interactions.js]
 [test_event_source.js]
 [test_getAll.js]
 [test_globalObjects_ipc.js]
--- a/xpcom/base/nsCycleCollector.cpp
+++ b/xpcom/base/nsCycleCollector.cpp
@@ -2719,22 +2719,23 @@ nsCycleCollector::CleanupAfterCollection
     }
     mIncrementalPhase = IdlePhase;
 }
 
 void
 nsCycleCollector::ShutdownCollect()
 {
     SliceBudget unlimitedBudget;
-    for (uint32_t i = 0; i < DEFAULT_SHUTDOWN_COLLECTIONS; ++i) {
-        NS_ASSERTION(i < NORMAL_SHUTDOWN_COLLECTIONS, "Extra shutdown CC");
+    uint32_t i;
+    for (i = 0; i < DEFAULT_SHUTDOWN_COLLECTIONS; ++i) {
         if (!Collect(ShutdownCC, unlimitedBudget, nullptr)) {
             break;
         }
     }
+    NS_ASSERTION(i < NORMAL_SHUTDOWN_COLLECTIONS, "Extra shutdown CC");
 }
 
 static void
 PrintPhase(const char *aPhase)
 {
 #ifdef DEBUG_PHASES
     printf("cc: begin %s on %s\n", aPhase,
            NS_IsMainThread() ? "mainthread" : "worker");