Bug 995162 - Rewrite the mechanism that (re)starts the OS.File worker. r=froydnj, a=lsblakk
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Wed, 16 Apr 2014 17:04:47 -0400
changeset 192184 0678e5469636
parent 192183 3c6892869d25
child 192185 e078f53fd234
push id3515
push userryanvm@gmail.com
push date2014-05-01 17:32 +0000
Treeherderresults
reviewersfroydnj, lsblakk
bugs995162
milestone30.0
Bug 995162 - Rewrite the mechanism that (re)starts the OS.File worker. r=froydnj, a=lsblakk
toolkit/components/osfile/modules/osfile_async_front.jsm
toolkit/components/osfile/modules/osfile_async_worker.js
--- a/toolkit/components/osfile/modules/osfile_async_front.jsm
+++ b/toolkit/components/osfile/modules/osfile_async_front.jsm
@@ -130,18 +130,18 @@ for (let [constProp, dirKey] of [
   });
 }
 
 /**
  * Return a shallow clone of the enumerable properties of an object.
  */
 let clone = SharedAll.clone;
 
-let worker = null;
 let Scheduler = {
+
   /**
    * |true| once we have sent at least one message to the worker.
    * This field is unaffected by resetting the worker.
    */
   launched: false,
 
   /**
    * |true| once shutdown has begun i.e. we should reject any
@@ -172,16 +172,35 @@ let Scheduler = {
   latestReceived: undefined,
 
   /**
    * A timer used to automatically shut down the worker after some time.
    */
   resetTimer: null,
 
   /**
+   * The worker to which to send requests.
+   *
+   * If the worker has never been created or has been reset, this is a
+   * fresh worker, initialized with osfile_async_worker.js.
+   *
+   * @type {PromiseWorker}
+   */
+  get worker() {
+    if (!this._worker) {
+      // Either the worker has never been created or it has been reset
+      this._worker = new PromiseWorker(
+	"resource://gre/modules/osfile/osfile_async_worker.js", LOG);
+    }
+    return this._worker;
+  },
+
+  _worker: null,
+
+  /**
    * Prepare to kill the OS.File worker after a few seconds.
    */
   restartTimer: function(arg) {
     let delay;
     try {
       delay = Services.prefs.getIntPref("osfile.reset_worker_delay");
     } catch(e) {
       // Don't auto-shutdown if we don't have a delay preference set.
@@ -207,34 +226,41 @@ let Scheduler = {
    *   would not cause leaks. Otherwise, assume that the worker will be shutdown
    *   through some other mean.
    */
   kill: function({shutdown, reset}) {
     return Task.spawn(function*() {
 
       yield this.queue;
 
-      if (!this.launched || this.shutdown || !worker) {
+      // Enter critical section: no yield in this block
+      // (we want to make sure that we remain the only
+      // request in the queue).
+
+      if (!this.launched || this.shutdown || !this._worker) {
         // Nothing to kill
         this.shutdown = this.shutdown || shutdown;
-        worker = null;
+        this._worker = null;
         return null;
       }
 
       // Deactivate the queue, to ensure that no message is sent
       // to an obsolete worker (we reactivate it in the |finally|).
       let deferred = Promise.defer();
       this.queue = deferred.promise;
 
+
+      // Exit critical section
+
       let message = ["Meta_shutdown", [reset]];
 
       try {
         Scheduler.latestReceived = [];
         Scheduler.latestSent = [Date.now(), ...message];
-        let promise = worker.post(...message);
+        let promise = this._worker.post(...message);
 
         // Wait for result
         let resources;
         try {
           resources = (yield promise).ok;
 
           Scheduler.latestReceived = [Date.now(), message];
         } catch (ex) {
@@ -263,17 +289,17 @@ let Scheduler = {
               openedDirectoryIterators.join("\n");
           }
 
           LOG("WARNING: File descriptors leaks detected.\n" + msg);
         }
 
         // Make sure that we do not leave an invalid |worker| around.
         if (killed || shutdown) {
-          worker = null;
+          this._worker = null;
         }
 
         this.shutdown = shutdown;
 
         return resources;
 
       } finally {
         // Resume accepting messages. If we have set |shutdown| to |true|,
@@ -310,54 +336,55 @@ let Scheduler = {
    * must be clonable.
    * @return {Promise} A promise conveying the result/error caused by
    * calling |method| with arguments |args|.
    */
   post: function post(method, ...args) {
     if (this.shutdown) {
       LOG("OS.File is not available anymore. The following request has been rejected.",
         method, args);
-      return Promise.reject(new Error("OS.File has been shut down."));
-    }
-    if (!worker) {
-      // Either the worker has never been created or it has been reset
-      worker = new PromiseWorker(
-        "resource://gre/modules/osfile/osfile_async_worker.js", LOG);
+      return Promise.reject(new Error("OS.File has been shut down. Rejecting post to " + method));
     }
     let firstLaunch = !this.launched;
     this.launched = true;
 
     if (firstLaunch && SharedAll.Config.DEBUG) {
       // If we have delayed sending SET_DEBUG, do it now.
-      worker.post("SET_DEBUG", [true]);
+      this.worker.post("SET_DEBUG", [true]);
     }
 
     // By convention, the last argument of any message may be an |options| object.
     let options;
     let methodArgs = args[0];
     if (methodArgs) {
       options = methodArgs[methodArgs.length - 1];
     }
-    return this.push(() => Task.spawn(function*() {
+    return this.push(Task.async(function*() {
+      if (this.shutdown) {
+	LOG("OS.File is not available anymore. The following request has been rejected.",
+	  method, args);
+	throw new Error("OS.File has been shut down. Rejecting request to " + method);
+      }
+
       Scheduler.latestReceived = null;
       if (OS.Constants.Sys.DEBUG) {
         // Update possibly memory-expensive debugging information
         Scheduler.latestSent = [Date.now(), method, ...args];
       } else {
         Scheduler.latestSent = [Date.now(), method];
       }
 
       // Don't kill the worker just yet
       Scheduler.restartTimer();
 
       let data;
       let reply;
       let isError = false;
       try {
-        data = yield worker.post(method, ...args);
+        data = yield this.worker.post(method, ...args);
         reply = data;
       } catch (error if error instanceof PromiseWorker.WorkerError) {
         reply = error;
         isError = true;
         throw EXCEPTION_CONSTRUCTORS[error.data.exn || "OSError"](error.data);
       } catch (error if error instanceof ErrorEvent) {
         reply = error;
         let message = error.message;
@@ -404,25 +431,26 @@ let Scheduler = {
       let durationMs = Math.max(0, data.durationMs);
       // Accumulate (or initialize) outExecutionDuration
       if (typeof options.outExecutionDuration == "number") {
         options.outExecutionDuration += durationMs;
       } else {
         options.outExecutionDuration = durationMs;
       }
       return data.ok;
-    }));
+    }.bind(this)));
   },
 
   /**
    * Post Telemetry statistics.
    *
    * This is only useful on first launch.
    */
   _updateTelemetry: function() {
+    let worker = this.worker;
     let workerTimeStamps = worker.workerTimeStamps;
     if (!workerTimeStamps) {
       // If the first call to OS.File results in an uncaught errors,
       // the timestamps are absent. As this case is a developer error,
       // let's not waste time attempting to extract telemetry from it.
       return;
     }
     let HISTOGRAM_LAUNCH = Services.telemetry.getHistogramById("OSFILE_WORKER_LAUNCH_MS");
@@ -1376,17 +1404,17 @@ AsyncShutdown.profileBeforeChange.addBlo
     } else {
       return Scheduler.queue;
     }
   },
   function getDetails() {
     let result = {
       launched: Scheduler.launched,
       shutdown: Scheduler.shutdown,
-      worker: !!worker,
+      worker: !!Scheduler._worker,
       pendingReset: !!Scheduler.resetTimer,
       latestSent: Scheduler.latestSent,
       latestReceived: Scheduler.latestReceived
     };
     // 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]);
--- a/toolkit/components/osfile/modules/osfile_async_worker.js
+++ b/toolkit/components/osfile/modules/osfile_async_worker.js
@@ -76,17 +76,17 @@ const EXCEPTION_NAMES = {
    let durationMs;
    try {
      let method = data.fun;
      LOG("Calling method", method);
      result = Agent[method].apply(Agent, data.args);
      LOG("Method", method, "succeeded");
    } catch (ex) {
      exn = ex;
-     LOG("Error while calling agent method", exn, exn.stack || "");
+     LOG("Error while calling agent method", exn, exn.moduleStack || exn.stack || "");
    }
 
    if (start) {
      // Record duration
      durationMs = Date.now() - start;
      LOG("Method took", durationMs, "ms");
    }