Bug 927560 - Shutdown osfile_async_worker.js when it's not needed;r=froydnj
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Thu, 24 Oct 2013 05:04:00 +0100
changeset 165766 e5c275ba558eac8c9e84ec175c207ff4690f7a42
parent 165765 0787526e0ed6c7d1c5cdb27daedfa2084589d114
child 165767 3efb38280835bd35cf7d903e0c98787c42351f07
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs927560
milestone27.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 927560 - Shutdown osfile_async_worker.js when it's not needed;r=froydnj
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
@@ -93,45 +93,50 @@ if (!("localProfileDir" in SharedAll.Con
   });
 }
 
 /**
  * Return a shallow clone of the enumerable properties of an object.
  */
 let clone = SharedAll.clone;
 
-let worker = new PromiseWorker(
-  "resource://gre/modules/osfile/osfile_async_worker.js", LOG);
+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
-   * message
+   * message, including resets.
    */
   shutdown: false,
 
   /**
    * The latest promise returned.
    */
   latestPromise: Promise.resolve("OS.File scheduler hasn't been launched yet"),
 
   post: function post(...args) {
+    if (this.shutdown) {
+      LOG("OS.File is not available anymore. The following request has been rejected.", 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);
+    }
     if (!this.launched && SharedAll.Config.DEBUG) {
       // If we have delayed sending SET_DEBUG, do it now.
       worker.post("SET_DEBUG", [true]);
     }
     this.launched = true;
-    if (this.shutdown) {
-      LOG("OS.File is not available anymore. The following request has been rejected.", args);
-      return Promise.reject(new Error("OS.File has been shut down."));
-    }
 
     // By convention, the last argument of any message may be an |options| object.
     let methodArgs = args[1];
     let options = methodArgs ? methodArgs[methodArgs.length - 1] : null;
     let promise = worker.post.apply(worker, args);
     return this.latestPromise = promise.then(
       function onSuccess(data) {
         // Check for duration and return result.
@@ -241,24 +246,23 @@ const PREF_OSFILE_TEST_SHUTDOWN_OBSERVER
  *
  * @param {bool=} shutdown If true or unspecified, reconfigure
  * the scheduler to reject further requests. Can be set to |false|
  * for testing purposes.
  * @return {promise} A promise satisfied once all pending messages
  * (including the shutdown warning message) have been answered.
  */
 function warnAboutUnclosedFiles(shutdown = true) {
-  if (!Scheduler.launched) {
+  if (!Scheduler.launched || !worker) {
     // Don't launch the scheduler on our behalf. If no message has been
     // sent to the worker, we can't have any leaking file/directory
     // descriptor.
     return null;
   }
-  // Send a "System_shutdown" message to the worker.
-  let promise = Scheduler.post("System_shutdown");
+  let promise = Scheduler.post("Meta_getUnclosedResources");
 
   // Configure the worker to reject any further message.
   if (shutdown) {
     Scheduler.shutdown = true;
   }
 
   return promise.then(function onSuccess(opened) {
     let msg = "";
@@ -990,40 +994,78 @@ DirectoryIterator.Entry = function Entry
   return value;
 };
 DirectoryIterator.Entry.prototype = Object.create(SysAll.AbstractEntry.prototype);
 
 DirectoryIterator.Entry.fromMsg = function fromMsg(value) {
   return new DirectoryIterator.Entry(value);
 };
 
+/**
+ * Flush all operations currently queued, then kill the underlying
+ * worker to save memory.
+ *
+ * @return {Promise}
+ * @reject {Error} If at least one file or directory iterator instance
+ * is still open and the worker cannot be killed safely.
+ */
+File.resetWorker = function() {
+  if (!Scheduler.launched || Scheduler.shutdown) {
+    // No need to reset
+    return Promise.resolve();
+  }
+  return Scheduler.post("Meta_reset").then(
+    function(wouldLeak) {
+      if (!wouldLeak) {
+        // No resource would leak, the worker was stopped.
+        worker = null;
+        return;
+      }
+      // Otherwise, resetting would be unsafe and has been canceled.
+      // Turn this into an error
+      let msg = "Cannot reset worker: ";
+      let {openedFiles, openedDirectoryIterators} = wouldLeak;
+      if (openedFiles.length > 0) {
+        msg += "The following files are still open:\n" +
+          openedFiles.join("\n");
+      }
+      if (openedDirectoryIterators.length > 0) {
+        msg += "The following directory iterators are still open:\n" +
+          openedDirectoryIterators.join("\n");
+      }
+      throw new Error(msg);
+    }
+  );
+};
+
+
 // Constants
 File.POS_START = SysAll.POS_START;
 File.POS_CURRENT = SysAll.POS_CURRENT;
 File.POS_END = SysAll.POS_END;
 
 // Exports
 File.Error = OSError;
 File.DirectoryIterator = DirectoryIterator;
 
 this.OS = {};
-OS.File = File;
-OS.Constants = SharedAll.Constants;
-OS.Shared = {
+this.OS.File = File;
+this.OS.Constants = SharedAll.Constants;
+this.OS.Shared = {
   LOG: SharedAll.LOG,
   Type: SysAll.Type,
   get DEBUG() {
     return SharedAll.Config.DEBUG;
   },
   set DEBUG(x) {
     return SharedAll.Config.DEBUG = x;
   }
 };
-Object.freeze(OS.Shared);
-OS.Path = Path;
+Object.freeze(this.OS.Shared);
+this.OS.Path = Path;
 
 
 // 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",
--- a/toolkit/components/osfile/modules/osfile_async_worker.js
+++ b/toolkit/components/osfile/modules/osfile_async_worker.js
@@ -62,20 +62,28 @@ if (this.Components) {
    }
 
    // Now, post a reply, possibly as an uncaught error.
    // We post this message from outside the |try ... catch| block
    // to avoid capturing errors that take place during |postMessage| and
    // built-in serialization.
    if (!exn) {
      LOG("Sending positive reply", result, "id is", id);
-     if (result instanceof Transfer) {
-       // Take advantage of zero-copy transfers
-       self.postMessage({ok: result.data, id: id, durationMs: durationMs},
-         result.transfers);
+     if (result instanceof Meta) {
+       if ("transfers" in result.meta) {
+         // Take advantage of zero-copy transfers
+         self.postMessage({ok: result.data, id: id, durationMs: durationMs},
+           result.meta.transfers);
+       } else {
+         self.postMessage({ok: result.data, id:id, durationMs: durationMs});
+       }
+       if (result.meta.shutdown || false) {
+         // Time to close the worker
+         self.close();
+       }
      } else {
        self.postMessage({ok: result, id:id, durationMs: durationMs});
      }
    } else if (exn == StopIteration) {
      // StopIteration cannot be serialized automatically
      LOG("Sending back StopIteration");
      self.postMessage({StopIteration: true, id: id, durationMs: durationMs});
    } else if (exn instanceof exports.OS.File.Error) {
@@ -187,58 +195,82 @@ if (this.Components) {
    }
    return f.call(file);
   };
 
   let Type = exports.OS.Shared.Type;
 
   let File = exports.OS.File;
 
- /**
-  * A constructor used to transfer data to the caller
-  * without copy.
-  *
-  * @param {*} data The data to return to the caller.
-  * @param {Array} transfers An array of Transferable
-  * values that should be moved instead of being copied.
-  *
-  * @constructor
-  */
-  let Transfer = function Transfer(data, transfers) {
-   this.data = data;
-   this.transfers = transfers;
+  /**
+   * A constructor used to return data to the caller thread while
+   * also executing some specific treatment (e.g. shutting down
+   * the current thread, transmitting data instead of copying it).
+   *
+   * @param {object=} data The data to return to the caller thread.
+   * @param {object=} meta Additional instructions, as an object
+   * that may contain the following fields:
+   * - {bool} shutdown If |true|, shut down the current thread after
+   *   having sent the result.
+   * - {Array} transfers An array of objects that should be transferred
+   *   instead of being copied.
+   *
+   * @constructor
+   */
+  let Meta = function Meta(data, meta) {
+    this.data = data;
+    this.meta = meta;
   };
 
  /**
   * The agent.
   *
   * It is in charge of performing method-specific deserialization
   * of messages, calling the function/method of OS.File and serializing
   * back the results.
   */
   let Agent = {
    // Update worker's OS.Shared.DEBUG flag message from controller.
-   SET_DEBUG: function SET_DEBUG (aDEBUG) {
+   SET_DEBUG: function(aDEBUG) {
      SharedAll.Config.DEBUG = aDEBUG;
    },
    // Return worker's current OS.Shared.DEBUG value to controller.
    // Note: This is used for testing purposes.
-   GET_DEBUG: function GET_DEBUG () {
+   GET_DEBUG: function() {
      return SharedAll.Config.DEBUG;
    },
-   // Report file descriptors leaks.
-   System_shutdown: function System_shutdown () {
+   Meta_getUnclosedResources: function() {
      // Return information about both opened files and opened
      // directory iterators.
      return {
        openedFiles: OpenedFiles.listOpenedResources(),
-       openedDirectoryIterators:
-         OpenedDirectoryIterators.listOpenedResources()
+       openedDirectoryIterators: OpenedDirectoryIterators.listOpenedResources()
      };
    },
+   Meta_reset: function() {
+     // Attempt to stop the worker. This fails if at least one
+     // resource is still open. Returns the list of files and
+     // directory iterators that cannot be closed safely (or undefined
+     // if there are no such files/directory iterators).
+     let openedFiles = OpenedFiles.listOpenedResources();
+     let openedDirectoryIterators =
+       OpenedDirectoryIterators.listOpenedResources();
+     let canShutdown = openedFiles.length == 0
+                         && openedDirectoryIterators.length == 0;
+     if (canShutdown) {
+       // Succeed. Shutdown the thread, nothing to return
+       return new Meta(null, {shutdown: true});
+     } else {
+       // Fail. Don't shutdown the thread, return info on resources
+       return {
+         openedFiles: openedFiles,
+         openedDirectoryIterators: openedDirectoryIterators
+       };
+     }
+   },
    // Functions of OS.File
    stat: function stat(path) {
      return exports.OS.File.Info.toMsg(
        exports.OS.File.stat(Type.path.fromMsg(path)));
    },
    getCurrentDirectory: function getCurrentDirectory() {
      return exports.OS.Shared.Type.path.toMsg(File.getCurrentDirectory());
    },
@@ -282,17 +314,23 @@ if (this.Components) {
 
      return {
        path: openedFile.path,
        file: resourceId
      };
    },
    read: function read(path, bytes, options) {
      let data = File.read(Type.path.fromMsg(path), bytes, options);
-     return new Transfer({buffer: data.buffer, byteOffset: data.byteOffset, byteLength: data.byteLength}, [data.buffer]);
+     return new Meta({
+         buffer: data.buffer,
+         byteOffset: data.byteOffset,
+         byteLength: data.byteLength
+     }, {
+       transfers: [data.buffer]
+     });
    },
    exists: function exists(path) {
      return File.exists(Type.path.fromMsg(path));
    },
    writeAtomic: function writeAtomic(path, buffer, options) {
      if (options.tmpPath) {
        options.tmpPath = Type.path.fromMsg(options.tmpPath);
      }
@@ -329,17 +367,23 @@ if (this.Components) {
        function do_stat() {
          return exports.OS.File.Info.toMsg(this.stat());
        });
    },
    File_prototype_read: function read(fd, nbytes, options) {
      return withFile(fd,
        function do_read() {
          let data = this.read(nbytes, options);
-         return new Transfer({buffer: data.buffer, byteOffset: data.byteOffset, byteLength: data.byteLength}, [data.buffer]);
+         return new Meta({
+             buffer: data.buffer,
+             byteOffset: data.byteOffset,
+             byteLength: data.byteLength
+         }, {
+           transfers: [data.buffer]
+         });
        }
      );
    },
    File_prototype_readTo: function readTo(fd, buffer, options) {
      return withFile(fd,
        function do_readTo() {
          return this.readTo(exports.OS.Shared.Type.voidptr_t.fromMsg(buffer),
          options);