Bug 777712 - Adding file descriptors leaks logging on web-workers-shutdown. r=yoric
authorYura Zenevich <yura.zenevich@gmail.com>
Wed, 20 Feb 2013 08:50:51 -0500
changeset 122453 cb220ef54f3d11fb7b9772afd6fd7ad79c566733
parent 122452 0c8e1a272c78100ec6082fc29334be4a93695bbd
child 122454 e8d73b8ba1a17e4a4b0dffcf59afa52c28510a61
push id24342
push userryanvm@gmail.com
push dateThu, 21 Feb 2013 13:05:06 +0000
treeherdermozilla-central@702d2814efbf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyoric
bugs777712
milestone22.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 777712 - Adding file descriptors leaks logging on web-workers-shutdown. r=yoric
toolkit/components/osfile/osfile_async_front.jsm
toolkit/components/osfile/osfile_async_worker.js
toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
--- a/toolkit/components/osfile/osfile_async_front.jsm
+++ b/toolkit/components/osfile/osfile_async_front.jsm
@@ -45,21 +45,22 @@ if (OS.Constants.Win) {
 let Type = OS.Shared.Type;
 
 // The library of promises.
 Components.utils.import("resource://gre/modules/commonjs/sdk/core/promise.js", this);
 
 // The implementation of communications
 Components.utils.import("resource://gre/modules/osfile/_PromiseWorker.jsm", this);
 
+Components.utils.import("resource://gre/modules/Services.jsm", this);
+
 // If profileDir is not available, osfile.jsm has been imported before the
 // profile is setup. In this case, we need to observe "profile-do-change"
 // and set OS.Constants.Path.profileDir as soon as it becomes available.
 if (!("profileDir" in OS.Constants.Path) || !("localProfileDir" in OS.Constants.Path)) {
-  Components.utils.import("resource://gre/modules/Services.jsm", this);
   let observer = function observer() {
     Services.obs.removeObserver(observer, "profile-do-change");
 
     let profileDir = Services.dirsvc.get("ProfD", Components.interfaces.nsIFile).path;
     OS.Constants.Path.profileDir = profileDir;
 
     let localProfileDir = Services.dirsvc.get("ProfLD", Components.interfaces.nsIFile).path;
     OS.Constants.Path.localProfileDir = localProfileDir;
@@ -121,16 +122,47 @@ Object.defineProperty(OS.Shared, "DEBUG"
     },
     set: function (newVal) {
         Scheduler.post("SET_DEBUG", [newVal]);
         DEBUG = newVal;
     }
 });
 
 /**
+ * An observer function to be used to monitor web-workers-shutdown events.
+ */
+let webWorkersShutdownObserver = function webWorkersShutdownObserver() {
+  // Send a "System_shutdown" message to the worker.
+  Scheduler.post("System_shutdown").then(function onSuccess(opened) {
+    let msg = "";
+    if (opened.openedFiles.length > 0) {
+      msg += "The following files are still opened:\n" +
+        opened.openedFiles.join("\n");
+    }
+    if (opened.openedDirectoryIterators.length > 0) {
+      msg += "The following directory iterators are still opened:\n" +
+        opened.openedDirectoryIterators.join("\n");
+    }
+    // Only log if file descriptors leaks detected.
+    if (msg) {
+      LOG("WARNING: File descriptors leaks detected.\n" + msg);
+    }
+  });
+};
+
+// Attaching an observer listening to the "web-workers-shutdown".
+Services.obs.addObserver(webWorkersShutdownObserver, "web-workers-shutdown",
+  false);
+// Attaching the same observer listening to the
+// "test.osfile.web-workers-shutdown".
+// Note: This is used for testing purposes.
+Services.obs.addObserver(webWorkersShutdownObserver,
+  "test.osfile.web-workers-shutdown", false);
+
+/**
  * Representation of a file, with asynchronous methods.
  *
  * @param {*} fdmsg The _message_ representing the platform-specific file
  * handle.
  *
  * @constructor
  */
 let File = function File(fdmsg) {
--- a/toolkit/components/osfile/osfile_async_worker.js
+++ b/toolkit/components/osfile/osfile_async_worker.js
@@ -109,16 +109,23 @@ if (this.Components) {
         *
         * @return {*} A unique identifier. For the moment, this is a number,
         * but this might not remain the case forever.
         */
        add: function(resource, info) {
          let id = this._idgen++;
          this._map.set(id, {resource:resource, info:info});
          return id;
+       },
+       /**
+        * Return a list of all open resources i.e. the ones still present in
+        * ResourceTracker's _map.
+        */
+       listOpenedResources: function listOpenedResources() {
+         return [resource.info.path for ([id, resource] of this._map)];
        }
      };
 
      /**
       * A map of unique identifiers to opened files.
       */
      let OpenedFiles = new ResourceTracker();
 
@@ -188,16 +195,26 @@ if (this.Components) {
        SET_DEBUG: function SET_DEBUG (aDEBUG) {
          exports.OS.Shared.DEBUG = aDEBUG;
        },
        // Return worker's current OS.Shared.DEBUG value to controller.
        // Note: This is used for testing purposes.
        GET_DEBUG: function GET_DEBUG () {
          return exports.OS.Shared.DEBUG;
        },
+       // Report file descriptors leaks.
+       System_shutdown: function System_shutdown () {
+         // Return information about both opened files and opened
+         // directory iterators.
+         return {
+           openedFiles: OpenedFiles.listOpenedResources(),
+           openedDirectoryIterators:
+             OpenedDirectoryIterators.listOpenedResources()
+         };
+       },
        // 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());
        },
@@ -217,18 +234,23 @@ if (this.Components) {
        },
        removeEmptyDir: function removeEmptyDir(path, options) {
          return File.removeEmptyDir(Type.path.fromMsg(path), options);
        },
        remove: function remove(path) {
          return File.remove(Type.path.fromMsg(path));
        },
        open: function open(path, mode, options) {
-         let file = File.open(Type.path.fromMsg(path), mode, options);
-         return OpenedFiles.add(file);
+         let filePath = Type.path.fromMsg(path);
+         let file = File.open(filePath, mode, options);
+         return OpenedFiles.add(file, {
+           // Adding path information to keep track of opened files
+           // to report leaks when debugging.
+           path: filePath
+         });
        },
        read: function read(path, bytes) {
          let data = File.read(Type.path.fromMsg(path), bytes);
          return new Transfer({buffer: data.buffer, byteOffset: data.byteOffset, byteLength: data.byteLength}, [data.buffer]);
        },
        exists: function exists(path) {
          return File.exists(Type.path.fromMsg(path));
        },
@@ -237,18 +259,23 @@ if (this.Components) {
            options.tmpPath = Type.path.fromMsg(options.tmpPath);
          }
          return File.writeAtomic(Type.path.fromMsg(path),
                                  Type.voidptr_t.fromMsg(buffer),
                                  options
                                 );
        },
        new_DirectoryIterator: function new_DirectoryIterator(path, options) {
-         let iterator = new File.DirectoryIterator(Type.path.fromMsg(path), options);
-         return OpenedDirectoryIterators.add(iterator);
+         let directoryPath = Type.path.fromMsg(path);
+         let iterator = new File.DirectoryIterator(directoryPath, options);
+         return OpenedDirectoryIterators.add(iterator, {
+           // Adding path information to keep track of opened directory
+           // iterators to report leaks when debugging.
+           path: directoryPath
+         });
        },
        // Methods of OS.File
        File_prototype_close: function close(fd) {
          return withFile(fd,
            function do_close() {
              try {
                return this.close();
              } finally {
--- a/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
+++ b/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js
@@ -136,16 +136,17 @@ let test = maketest("Main", function mai
     yield test_read_write();
     yield test_read_write_all();
     yield test_position();
     yield test_copy();
     yield test_mkdir();
     yield test_iter();
     yield test_exists();
     yield test_debug_test();
+    yield test_system_shutdown();
     info("Test is over");
     SimpleTest.finish();
   });
 });
 
 /**
  * A file that we know exists and that can be used for reading.
  */
@@ -686,9 +687,60 @@ let test_debug_test = maketest("debug_te
     let originalPref = OS.Shared.DEBUG;
     toggleDebugTest(true);
     // Execution of OS.File.exist method will trigger OS.File.LOG several times.
     let fileExists = yield OS.File.exists(EXISTING_FILE);
     toggleDebugTest(false);
     // Restore DEBUG to its original.
     OS.Shared.DEBUG = originalPref;
   });
+});
+
+/**
+ * Test logging of file descriptors leaks.
+ */
+let test_system_shutdown = maketest("system_shutdown", function system_shutdown(test) {
+  return Task.spawn(function () {
+    // Save original DEBUG value.
+    let originalDebug = OS.Shared.DEBUG;
+    // Create a console listener.
+    function getConsoleListener(resource) {
+      let consoleListener = {
+        observe: function (aMessage) {
+          // Ignore unexpected messages.
+          if (!(aMessage instanceof Components.interfaces.nsIConsoleMessage)) {
+            return;
+          }
+          if (aMessage.message.indexOf("TEST OS Controller WARNING") < 0) {
+            return;
+          }
+          test.ok(aMessage.message.indexOf("WARNING: File descriptors leaks " +
+            "detected.") >= 0, "File descriptors leaks are logged correctly.");
+          test.ok(aMessage.message.indexOf(resource) >= 0,
+            "Leaked resource is correctly listed in the log.");
+          toggleDebugTest(false, resource, consoleListener);
+        }
+      };
+      return consoleListener;
+    }
+    // Set/Unset testing flags and Services.console listener.
+    function toggleDebugTest(startDebug, resource, consoleListener) {
+      Services.console[startDebug ? "registerListener" : "unregisterListener"](
+        consoleListener || getConsoleListener(resource));
+      OS.Shared.TEST = startDebug;
+      // If pref is false, restore DEBUG to its original.
+      OS.Shared.DEBUG = startDebug || originalDebug;
+    }
+
+    let currentDir = yield OS.File.getCurrentDirectory();
+    let iterator = new OS.File.DirectoryIterator(currentDir);
+    toggleDebugTest(true, currentDir);
+    Services.obs.notifyObservers(null, "test.osfile.web-workers-shutdown",
+      null);
+    yield iterator.close();
+
+    let openedFile = yield OS.File.open(EXISTING_FILE);
+    toggleDebugTest(true, EXISTING_FILE);
+    Services.obs.notifyObservers(null, "test.osfile.web-workers-shutdown",
+      null);
+    yield openedFile.close();
+  });
 });
\ No newline at end of file