Bug 1008944 - Expose AsyncShutdown barriers for OS.File. r=froydnj, a=sledru
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Tue, 17 Jun 2014 15:11:00 -0400
changeset 207693 6a25989f0c6c086e495de880f9e19cc883fd7310
parent 207692 fad78d49fe076822234a0f6391ec5a424a413a4f
child 207694 3c6ae6dbf114ff51f7db170d373dba420c4db920
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, sledru
bugs1008944
milestone32.0a2
Bug 1008944 - Expose AsyncShutdown barriers for OS.File. r=froydnj, a=sledru
toolkit/components/crashmonitor/CrashMonitor.jsm
toolkit/components/osfile/modules/osfile_async_front.jsm
toolkit/crashreporter/test/unit/test_crash_AsyncShutdown.js
--- a/toolkit/components/crashmonitor/CrashMonitor.jsm
+++ b/toolkit/components/crashmonitor/CrashMonitor.jsm
@@ -160,19 +160,20 @@ this.CrashMonitor = {
     // called after receiving it
     CrashMonitorInternal.checkpoints["profile-after-change"] = true;
 
     NOTIFICATIONS.forEach(function (aTopic) {
       Services.obs.addObserver(this, aTopic, false);
     }, this);
 
     // Add shutdown blocker for profile-before-change
-    AsyncShutdown.profileBeforeChange.addBlocker(
+    OS.File.profileBeforeChange.addBlocker(
       "CrashMonitor: Writing notifications to file after receiving profile-before-change",
-      CrashMonitorInternal.profileBeforeChangeDeferred.promise
+      CrashMonitorInternal.profileBeforeChangeDeferred.promise,
+      () => this.checkpoints
     );
 
     CrashMonitorInternal.initialized = true;
     if (Services.metro && Services.metro.immersive) {
       OS.File.makeDir(OS.Path.join(OS.Constants.Path.profileDir, "metro"));
     }
     return promise;
   },
--- a/toolkit/components/osfile/modules/osfile_async_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_async_front.jsm
@@ -449,16 +449,17 @@ let Scheduler = {
       Scheduler.restartTimer();
 
 
       let data;
       let reply;
       let isError = false;
       try {
         try {
+          Scheduler.Debugging.messagesSent++;
           data = yield this.worker.post(method, ...args);
         } finally {
           Scheduler.Debugging.messagesReceived++;
         }
         reply = data;
       } catch (error) {
         reply = error;
         isError = true;
@@ -603,17 +604,28 @@ if (SharedAll.Config.DEBUG && Scheduler.
 const WEB_WORKERS_SHUTDOWN_TOPIC = "web-workers-shutdown";
 
 // Preference used to configure test shutdown observer.
 const PREF_OSFILE_TEST_SHUTDOWN_OBSERVER =
   "toolkit.osfile.test.shutdown.observer";
 
 AsyncShutdown.webWorkersShutdown.addBlocker(
   "OS.File: flush pending requests, warn about unclosed files, shut down service.",
-  () => Scheduler.kill({reset: false, shutdown: true})
+  Task.async(function*() {
+    // Give clients a last chance to enqueue requests.
+    yield Barriers.shutdown.wait({crashAfterMS: null});
+
+    // Wait until all requests are complete and kill the worker.
+    yield Scheduler.kill({reset: false, shutdown: true});
+  }),
+  () => {
+    let details = Barriers.getDetails();
+    details.clients = Barriers.shutdown.state;
+    return details;
+  }
 );
 
 
 // Attaching an observer for PREF_OSFILE_TEST_SHUTDOWN_OBSERVER to enable or
 // disable the test shutdown event observer.
 // Note: By default the PREF_OSFILE_TEST_SHUTDOWN_OBSERVER is unset.
 // Note: This is meant to be used for testing purposes only.
 Services.prefs.addObserver(PREF_OSFILE_TEST_SHUTDOWN_OBSERVER,
@@ -1557,51 +1569,62 @@ this.OS.Path = Path;
 
 // Returns a resolved promise when all the queued operation have been completed.
 Object.defineProperty(OS.File, "queue", {
   get: function() {
     return Scheduler.queue;
   }
 });
 
-// Auto-flush OS.File during profile-before-change. This ensures that any I/O
-// that has been queued *before* profile-before-change is properly completed.
-// To ensure that I/O queued *during* profile-before-change is completed,
-// clients should register using AsyncShutdown.addBlocker.
-AsyncShutdown.profileBeforeChange.addBlocker(
-  "OS.File: flush I/O queued before profile-before-change",
-  // Wait until the latest currently enqueued promise is satisfied/rejected
-  function() {
-    let DEBUG = false;
-    try {
-      DEBUG = Services.prefs.getBoolPref("toolkit.osfile.debug.failshutdown");
-    } catch (ex) {
-      // Ignore
-    }
-    if (DEBUG) {
-      // Return a promise that will never be satisfied
-      return Promise.defer().promise;
-    } else {
-      return Scheduler.queue;
-    }
-  },
-  function getDetails() {
+/**
+ * Shutdown barriers, to let clients register to be informed during shutdown.
+ */
+let Barriers = {
+  profileBeforeChange: new AsyncShutdown.Barrier("OS.File: Waiting for clients before profile-before-shutdown"),
+  shutdown: new AsyncShutdown.Barrier("OS.File: Waiting for clients before full shutdown"),
+  /**
+   * Return the shutdown state of OS.File
+   */
+  getDetails: function() {
     let result = {
       launched: Scheduler.launched,
       shutdown: Scheduler.shutdown,
       worker: !!Scheduler._worker,
       pendingReset: !!Scheduler.resetTimer,
       latestSent: Scheduler.Debugging.latestSent,
       latestReceived: Scheduler.Debugging.latestReceived,
       messagesSent: Scheduler.Debugging.messagesSent,
       messagesReceived: Scheduler.Debugging.messagesReceived,
       messagesQueued: Scheduler.Debugging.messagesQueued,
-      DEBUG: SharedAll.Config.DEBUG
+      DEBUG: SharedAll.Config.DEBUG,
     };
     // Convert dates to strings for better readability
     for (let key of ["latestSent", "latestReceived"]) {
       if (result[key] && typeof result[key][0] == "number") {
         result[key][0] = Date(result[key][0]);
       }
     }
     return result;
   }
+};
+
+File.profileBeforeChange = Barriers.profileBeforeChange.client;
+File.shutdown = Barriers.shutdown.client;
+
+// Auto-flush OS.File during profile-before-change. This ensures that any I/O
+// that has been queued *before* profile-before-change is properly completed.
+// To ensure that I/O queued *during* profile-before-change is completed,
+// clients should register using AsyncShutdown.addBlocker.
+AsyncShutdown.profileBeforeChange.addBlocker(
+  "OS.File: flush I/O queued before profile-before-change",
+  Task.async(function*() {
+    // Give clients a last chance to enqueue requests.
+    yield Barriers.profileBeforeChange.wait({crashAfterMS: null});
+
+    // Wait until all currently enqueued requests are completed.
+    yield Scheduler.queue;
+  }),
+  () => {
+    let details = Barriers.getDetails();
+    details.clients = Barriers.profileBeforeChange.state;
+    return details;
+  }
 );
--- a/toolkit/crashreporter/test/unit/test_crash_AsyncShutdown.js
+++ b/toolkit/crashreporter/test/unit/test_crash_AsyncShutdown.js
@@ -35,28 +35,30 @@ function after_crash(mdump, extra) {
 }
 
 // Test that AsyncShutdown + OS.File reports errors correctly, in a case in which
 // the latest operation succeeded
 
 function setup_osfile_crash_noerror() {
   Components.utils.import("resource://gre/modules/Services.jsm", this);
   Components.utils.import("resource://gre/modules/osfile.jsm", this);
+  Components.utils.import("resource://gre/modules/Promise.jsm", this);
 
-  Services.prefs.setBoolPref("toolkit.osfile.debug.failshutdown", true);
   Services.prefs.setIntPref("toolkit.asyncshutdown.crash_timeout", 1);
   Services.prefs.setBoolPref("toolkit.osfile.native", false);
 
+  OS.File.profileBeforeChange.addBlocker("Adding a blocker that will never be resolved", () => Promise.defer().promise);
   OS.File.getCurrentDirectory();
+
   Services.obs.notifyObservers(null, "profile-before-change", null);
   dump("Waiting for crash\n");
 };
 
 function after_osfile_crash_noerror(mdump, extra) {
-  do_print("after OS.File crash: " + JSON.stringify(extra.AsyncShutdownTimeout));
+  do_print("after OS.File crash: " + extra.AsyncShutdownTimeout);
   let info = JSON.parse(extra.AsyncShutdownTimeout);
   let state = info.conditions[0].state;
   do_print("Keys: " + Object.keys(state).join(", "));
   do_check_eq(info.phase, "profile-before-change");
   do_check_true(state.launched);
   do_check_false(state.shutdown);
   do_check_true(state.worker);
   do_check_true(!!state.latestSent);
@@ -64,28 +66,30 @@ function after_osfile_crash_noerror(mdum
 }
 
 // Test that AsyncShutdown + OS.File reports errors correctly, in a case in which
 // the latest operation failed
 
 function setup_osfile_crash_exn() {
   Components.utils.import("resource://gre/modules/Services.jsm", this);
   Components.utils.import("resource://gre/modules/osfile.jsm", this);
+  Components.utils.import("resource://gre/modules/Promise.jsm", this);
 
-  Services.prefs.setBoolPref("toolkit.osfile.debug.failshutdown", true);
   Services.prefs.setIntPref("toolkit.asyncshutdown.crash_timeout", 1);
   Services.prefs.setBoolPref("toolkit.osfile.native", false);
 
+  OS.File.profileBeforeChange.addBlocker("Adding a blocker that will never be resolved", () => Promise.defer().promise);
   OS.File.read("I do not exist");
+
   Services.obs.notifyObservers(null, "profile-before-change", null);
   dump("Waiting for crash\n");
 };
 
 function after_osfile_crash_exn(mdump, extra) {
-  do_print("after OS.File crash: " + JSON.stringify(extra.AsyncShutdownTimeout));
+  do_print("after OS.File crash: " + extra.AsyncShutdownTimeout);
   let info = JSON.parse(extra.AsyncShutdownTimeout);
   let state = info.conditions[0].state;
   do_print("Keys: " + Object.keys(state).join(", "));
   do_check_eq(info.phase, "profile-before-change");
   do_check_false(state.shutdown);
   do_check_true(state.worker);
   do_check_true(!!state.latestSent);
   do_check_eq(state.latestSent[1], "read");