Skip to content

Bug 764840: Fix loader.js JSM leak. #465

Merged
merged 0 commits into from Jun 27, 2012

2 participants

@ochameau
Mozilla member
@Gozala Gozala commented on an outdated diff Jun 15, 2012
packages/api-utils/lib/cuddlefish.js
@@ -35,8 +38,9 @@ require('api-utils/addon/runner') // Otherwise CFX will stip out addon/runner.js
// used since regular require is not available at loader bootstrap.
const loaderURI = module.uri.replace(/\/[^\/]*$/, '/loader.js');
const loaderModule = require(loaderURI);
-const { Loader: BaseLoader, Require, Sandbox, resolveURI, evaluate, load,
- Module, unload, override, descriptor, main } = loaderModule;
+const { Loader: BaseLoader, unload: BaseUnload,
@Gozala
Mozilla member
Gozala added a note Jun 15, 2012

I think name should be unloadBase or baseUnload. We use BaseLoader because we had Loader there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Gozala Gozala commented on an outdated diff Jun 15, 2012
packages/api-utils/lib/cuddlefish.js
@@ -90,4 +93,13 @@ function Loader(options) {
Loader.prototype = null;
exports.Loader = Object.freeze(Loader);
+function unload() {
@Gozala
Mozilla member
Gozala added a note Jun 15, 2012

I'm not sure this is correct behavior. loader's unload passed instances of a loader and not the loader module itself. In this case you seem to unload loader module along with an individual loader instance. Which also means that unloading any instance of a loader will unload the whole module thing.

Only reason why it still works for tests is a live references to a loader instances. If you really want to unload this module then just do it from the shimmed require itself. I don't know what are the implications of that, but they are same as ones here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Gozala Gozala was assigned Jun 15, 2012
@ochameau ochameau merged commit cd13dec into mozilla:master Jun 27, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.