Skip to content
This repository has been archived by the owner on Feb 26, 2022. It is now read-only.

Bug 764840: Fix loader leak by using Sandboxes and nuking them instead of JSM. #533

Merged

Conversation

ochameau
Copy link
Contributor

Next try, after PR #473.

I tried to merge cuddlefish into bootstrap but it ended up being complicated because of manifest.
We have to put fake require statement in loader.js and I don't think that we want such noise in toolkit's loader.js ... (That's to be able to load addon/runner from bootstrap)

So I kept cuddlefish.js and switched to Sandboxes for it and loader.js.

https://bugzilla.mozilla.org/show_bug.cgi?id=764840

function loadSandbox(uri) {
let proto = { sandboxPrototype: { loadSandbox: loadSandbox } };
let module = Cu.Sandbox(systemPrincipal, proto);
module.__URI__ = uri;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather prefer module.module = { uri: uri, exports: {} }

// correctly
sandbox.exports = {};
sandbox.module = { uri: uri, exports: sandbox.exports };
sandbox.require = function () {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's such a good idea, it may cause surprising race conditions. Either don't expose require or throw some clear message from it.

ochameau added a commit that referenced this pull request Sep 1, 2012
…ader

Bug 764840: Fix loader leak by using Sandboxes and nuking them instead of JSM. r=@Gozala
@ochameau ochameau merged commit 274ed55 into mozilla:master Sep 1, 2012
ochameau added a commit that referenced this pull request Sep 2, 2012
…s-for-loader"

This reverts commit 274ed55, reversing
changes made to 8f03c79.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants