From 1af9ac6db625dee8703410a977725f41ee52b022 Mon Sep 17 00:00:00 2001 From: Yura Zenevich Date: Wed, 20 Feb 2013 08:50:51 -0500 Subject: [PATCH] Bug 777712 - Adding file descriptors leaks logging on web-workers-shutdown. r=yoric --- .../components/osfile/osfile_async_front.jsm | 34 +++++++++++- .../components/osfile/osfile_async_worker.js | 35 +++++++++++-- .../tests/mochi/main_test_osfile_async.js | 52 +++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/toolkit/components/osfile/osfile_async_front.jsm b/toolkit/components/osfile/osfile_async_front.jsm index 910a08758eaa..7329cc0caa88 100644 --- a/toolkit/components/osfile/osfile_async_front.jsm +++ b/toolkit/components/osfile/osfile_async_front.jsm @@ -50,11 +50,12 @@ Components.utils.import("resource://gre/modules/commonjs/sdk/core/promise.js", t // 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"); @@ -125,6 +126,37 @@ Object.defineProperty(OS.Shared, "DEBUG", { } }); +/** + * 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. * diff --git a/toolkit/components/osfile/osfile_async_worker.js b/toolkit/components/osfile/osfile_async_worker.js index bfc52767984f..94128f986788 100644 --- a/toolkit/components/osfile/osfile_async_worker.js +++ b/toolkit/components/osfile/osfile_async_worker.js @@ -114,6 +114,13 @@ if (this.Components) { 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)]; } }; @@ -193,6 +200,16 @@ if (this.Components) { 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( @@ -222,8 +239,13 @@ if (this.Components) { 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); @@ -242,8 +264,13 @@ if (this.Components) { ); }, 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) { diff --git a/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js b/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js index 0f06abf2c062..d4386f5d54f0 100644 --- a/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js +++ b/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js @@ -141,6 +141,7 @@ let test = maketest("Main", function main(test) { yield test_iter(); yield test_exists(); yield test_debug_test(); + yield test_system_shutdown(); info("Test is over"); SimpleTest.finish(); }); @@ -691,4 +692,55 @@ let test_debug_test = maketest("debug_test", function debug_test(test) { // 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