I'm currently trying to use best effort approach in Jetpack in order to maximize our chances to *not* leak! One possible improvement is to immediatly call `Cu.unload` right after `Cu.import`. But we are having some issues as global variables of the JSM seems to be collected right after the call to `unload`. # Module.jsm: # First, a JSM module hosted on a resource:// URI # With one exported method which call another private-global one const EXPORTED_SYMBOLS = ["foo"]; function foo() { bar(); } function bar() { } # test.js: # And the unit test itself let Cu = Components.utils; let uri = "resource://path/to/module.jsm" let imports = {}; Cu.import(uri, imports); imports.foo(); Cu.unload(uri); // This call will throw with "bar is not a function". // And works if you comment the call to `unload` imports.foo(); Note that we can replace `bar();` with `dump("foo");`. We will face the same issue, `dump` disappear too.

See the discussion from bug 481603 comment 4 for why this happens the way it does right now. I think it'd be better to leave the old scope alive and just eligible for GC generally but I don't know enough about what risks we run there.

No kidding! You've actually stumbled on the last remaining (or next-to-last-remaining) use of JS_ClearScope left in the tree! That's a dastardly jit-crashing function that we've been trying to kill for years! Peter, now that your work landed, we can just kill this, right? And maybe kill the API altogether?

We may have three distinct goals here: 1/ My current goal is to avoid leaking JSM because of the JSM cache in this `mImports` list. Actually the more I think about this, the more I think JSM is a bad option at first place. It might just be easier to use a sandbox... Here we do not care much about loading a fresh version, I'm only loading it once. And I do not want to free anything after the call to unload. 2/ One other goal would be to just allow reloading a new version of a JSM. We do not care much about the old module, we mainly want to be able to load a fresh version. 3/ A last goal would be to ensure that everything is collected on a given JSM. Here JS_ClearScope makes sense! We may even want to go futher and nuke CCW like what has been done in bug 695480. (I'm trying to figure out if it makes sense to do the same for sandboxes in bug 769273.) I'd say that we shouldn't be using JSM at first place, but documentation should be updated: http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/idl/xpcIJSModuleLoader.idl#61 And unload may be even more agressive on cleaning up stuff.

(In reply to Alexandre Poirot (:ochameau) from comment #3) > 3/ A last goal would be to ensure that everything is collected on a given > JSM. Here JS_ClearScope makes sense! We may even want to go futher and nuke > CCW like what has been done in bug 695480. (I'm trying to figure out if it > makes sense to do the same for sandboxes in bug 769273.) No, JS_ClearScope needs to go away. Nuking CCWs should be the primary approach here.

Since we have nukeSandbox, can I just close this one? Or do we still need it?

We stopped using JSM in jetpack because of this bug, but it might still be worth having it fixed for other m-c codebase. According to comment 4, it looks like some dark stuff is being done and it should be fixed. But again, it isn't a jetpack blocker anymore.

I would just like to say that this affects me as well. First I would like to point out that the docs disagree with the current behavior. Either the docs, or implementation should be upgraded to agree. The Cu.unload page (https://developer.mozilla.org/en-US/docs/Components.utils.unload) reads. > Once this method has been called, references to the module will continue to work... I can only imagine that many, if not most modules do not work after you nuke their global object. Now a bit of opinion. From the documentation I gathered the picture that unloading a module pretty much just removed the "cache" entry, (and any other "system" roots) which allowed the module to be collected once it is no longer referenced by client code (probably the code that imported it in the first place). I would argue that this behavior is better than the current behavior because in a garbage collected language I wouldn't expect things I can access to "disappear". This has caused a number of hard to find bugs in my code and working around this is a real pain as well. Plus the further issue is that it doesn't really help reduce memory anyways as much of the memory/objects/... from the scope will be kept due to other references. And, due to the likely crash of destructors when their global is cleared can make it hard to properly release the memory.

I don't think anyone's disputing that we should remove the JS_ClearScope calls (which have now been renamed to JS_SetAllNonReservedSlotsToUndefined). It's just that it requires some engineering and bug-hunting, since removing it causes us to leak (try removing the call to JS_SetAllNonReservedSlotsToUndefined in mozJSComponentLoader.h and running a single mochitest). I don't have time to work on it myself, but am happy to mentor anyone who wants to look at it. Some of the CC people might also want to take a look, since it might indicate CC issues that we should fix.

Sorry. I misunderstood some of the above comments. I have added a note on the wiki page to document that it does not work as intended.

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.