sandboxes vs Components.utils.import() for the Addon SDK

[Nickolay Ponomarev]

Hi all,

After seeing Irakli's work on Components.utils.import()-based loader <https://plus.google.com/106343137603240143566/posts/RdDGumbnLmf?hl=en> I've been thinking if maybe sandboxes were a better match for jetpack's modules and page-mods.

Here's a brief comparison of capabilities I put together: https://wiki.mozilla.org/User:Asqueella/Sandboxes

As far as I know, the main problem with sandboxes is that the code has to be read into a JS string and then passed to evalInSandbox() by the loader, with no caching of compiled code.

So maybe adding sandbox.loadScript(url) with the necessary caching would be better than trying to use Cu.import? The loader could also take advantage of this method only when it exists, making older platform versions easier to support.

BTW, it seems that for sandboxes with the system principal this functionality is already in Firefox 8:
https://bugzilla.mozilla.org/show_bug.cgi?id=648125
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#3457

3461 Components.utils.evalInSandbox(
3462    "Components.classes['@mozilla.org/moz/jssubscript-loader;1'] \
3463      .createInstance(Components.interfaces.mozIJSSubScriptLoader) \
3464      .loadSubScript(__SCRIPT_URI_SPEC__);", .........);

Is there anything I'm missing? Does anyone know the performance/memory characteristics of Sandbox vs Cu.import modules?

Nickolay

[Irakli Gozalishvili]

Hi Nickolay,

On Tuesday, 2011-07-19 at 22:51 , Nickolay Ponomarev wrote:

Hi all,

After seeing Irakli's work on Components.utils.import()-based loader <https://plus.google.com/106343137603240143566/posts/RdDGumbnLmf?hl=en> I've been thinking if maybe sandboxes were a better match for jetpack's modules and page-mods.

 

Feel free to jump in https://github.com/mozilla/addon-sdk/pull/212 

Here's a brief comparison of capabilities I put together: https://wiki.mozilla.org/User:Asqueella/Sandboxes

Here is few comments on that:

Run code with non-system principal

All jetpack modules run with system principal anyway.

Multiple "instances" of the same code (page-mods, unit-test)

 

I guess we could create multiple modules instances, but as far as I know we don't do it.

 Unloadable

Do you suggest that sandboxes may be destroyed in some way ? 

Many extra globals: atob, btoa, File, __LOCATION__, __URI__, EXPORTED_SYMBOLS

Actually having atob / boat is awesome, as we won't have to use hidden window for that :D (I did not knew they were there)!!!

I agree that those globals are inconvenient, but they can be easily shadowed:

https://github.com/mozilla/addon-sdk/pull/212/files#L1R207

 

As far as I know, the main problem with sandboxes is that the code has to be read into a JS string and then passed to evalInSandbox() by the loader, with no caching of compiled code.So maybe adding sandbox.loadScript(url) with the necessary caching would be better than trying to use Cu.import? The loader could also take advantage of this method only when it exists, making older platform versions easier to support.

So maybe adding sandbox.loadScript(url) with the necessary caching would be better than trying to use Cu.import? The loader could also take advantage of this method only when it exists, making older platform versions easier to support.

 

I don't think compiled code cacheing is relevant, as module from the same url is never executed more than once. We cache module exports so that second require('foo') will return exports from the cache. Also note sure you're aware, SDK modules are bundled with each add-on and are mapped to a diff resource URIs, which means that exact same sdk module is executed per addon.

 

BTW, it seems that for sandboxes with the system principal this functionality is already in Firefox 8:
https://bugzilla.mozilla.org/show_bug.cgi?id=648125
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/XPIProvider.jsm#3457

3461 Components.utils.evalInSandbox(
3462    "Components.classes['@mozilla.org/moz/jssubscript-loader;1'] \
3463      .createInstance(Components.interfaces.mozIJSSubScriptLoader) \
3464      .loadSubScript(__SCRIPT_URI_SPEC__);", .........);

Is there anything I'm missing? Does anyone know the performance/memory characteristics of Sandbox vs Cu.import modules?

Main reason I've started with this loader is to simplify complexity of the current loader and offloading as much to the platform as possible. So ideas is to replace all of this:

https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/app-extension/bootstrap.js
https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/app-extension/components/harness.js
https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/lib/cuddlefish.js

https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/lib/securable-module.js

with this:

https://github.com/mozilla/addon-sdk/pull/212/files

Also, I guess simplified module loader can be modified to use sandboxes instead, if it will turn out to be a better option.

BTW there was a thread about this:   

https://groups.google.com/forum/#!topic/mozilla-labs-jetpack/LaYkFpTcMqE

Nickolay

--
You received this message because you are subscribed to the Google Groups "mozilla-labs-jetpack" group.
To post to this group, send email to mozilla-la...@googlegroups.com.
To unsubscribe from this group, send email to mozilla-labs-jet...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/mozilla-labs-jetpack?hl=en.

[Dave Townsend]

Couple of quick thoughts here:

On Tuesday, July 19, 2011 5:52:45 PM UTC-7, gozala wrote:

On Tuesday, 2011-07-19 at 22:51 , Nickolay Ponomarev wrote:Run code with non-system principal

All jetpack modules run with system principal anyway.

I think in general you want the system principal right now. Presumably you could also load a couple of small parts of code into a sandbox somehow should you require a non-system principal for a specific set of code.

Multiple "instances" of the same code (page-mods, unit-test)

 

I guess we could create multiple modules instances, but as far as I know we don't do it.

You win on memory usage and load times by not doing it, we should keep that.

 Unloadable

Do you suggest that sandboxes may be destroyed in some way ? 

To my knowledge there is no explicit way to destroy a sandbox from JS (it is possible from C++). You can only drop all references to the sandbox and wait for it to be garbage collected. JS modules on the other hand get destroyed by the new Components.utils.unload functionality.
 

Many extra globals: atob, btoa, File, __LOCATION__, __URI__, EXPORTED_SYMBOLS

Actually having atob / boat is awesome, as we won't have to use hidden window for that :D (I did not knew they were there)!!!

I agree that those globals are inconvenient, but they can be easily shadowed:

https://github.com/mozilla/addon-sdk/pull/212/files#L1R207

Sandboxes on the other had apparently have a bunch of functionality missing. I wish I could be more explicit and point you to a bug for it but some developer was complaining to me about it because his bootstrap.js script couldn't use some things he wanted.

As far as I know, the main problem with sandboxes is that the code has to be read into a JS string and then passed to evalInSandbox() by the loader, with no caching of compiled code.So maybe adding sandbox.loadScript(url) with the necessary caching would be better than trying to use Cu.import? The loader could also take advantage of this method only when it exists, making older platform versions easier to support.

So maybe adding sandbox.loadScript(url) with the necessary caching would be better than trying to use Cu.import? The loader could also take advantage of this method only when it exists, making older platform versions easier to support.

 

I don't think compiled code cacheing is relevant, as module from the same url is never executed more than once. We cache module exports so that second require('foo') will return exports from the cache. Also note sure you're aware, SDK modules are bundled with each add-on and are mapped to a diff resource URIs, which means that exact same sdk module is executed per addon.

I actually think that (with proper version handling) allowing multiple add-ons to essentially access the same SDK modules would be a great way to save the performance hit per add-on. Perhaps that doesn't make a difference once we're in an e10s world though.

[Alexandre poirot]

There is an ongoing discussion about memory used by jetpack modules.
There is a significant overhead using sandboxes:
  https://bugzilla.mozilla.org/show_bug.cgi?id=672443
I think that same overhead apply to jsm.

From my point of view, I think that the main drawback of Cu.import is the lack of control of the JS context. We can't set our custom principal, nor JS version.

So it is obvious that we have to improve our performances, first by removing all IO from JS code and let c++ code handle script reading. We have to get rid of "evalInSandbox(hugeScriptString)"! Irakli proposal handle this nicely.

Then, from what I know, we have only two options: Cu.import or loadSubScript.
In both cases we will end up with module having system principal. As "evalInSandbox(...loadSubScript...)" is only going to work with a sandbox having system principal.
Having only modules running in system principal is not a big deal now, as it is already the case.
But we should give a try to limit privilege of modules in order to ensure better security reviews.
I have lot of comments still waiting to be published in the following bug:
Bug 636145 - By default chrome capabilities in modules
https://bugzilla.mozilla.org/show_bug.cgi?id=636145
I tried to limit system principal to modules that require(chrome), I end up facing wrappers issues that are automatically built between privilege and non-privilege code.

About cache, I used to think that the one used in Cu.import would be a drawback for jetpack.
But I realised, by reading the name of this cache "StartupCache", that it can increase startup performance for jetpack addons! So even if we don't share any piece of code, it may still improve startup performance by avoiding JS "compilation" on startup.
http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/loader/mozJSComponentLoader.cpp#850
http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/loader/mozJSComponentLoader.cpp#952

Finally, I would say that the priority is to simplify our codebase with proposal like Irakli's loader, then improving performance is going to be wayyyy simplier!

2011/7/19 Nickolay Ponomarev <asqu...@gmail.com>

[Nickolay Ponomarev]

Hi,

Thanks for your reply! I'll reply both to you and (indirectly) Dave here, since you raise similar points.

On Wed, Jul 20, 2011 at 4:52 AM, Irakli Gozalishvili <rfo...@gmail.com> wrote:

On Tuesday, 2011-07-19 at 22:51 , Nickolay Ponomarev wrote:

Here's a brief comparison of capabilities I put together: https://wiki.mozilla.org/User:Asqueella/Sandboxes

Here is few comments on that:

Run code with non-system principal

All jetpack modules run with system principal anyway.

 

At this time yes, but there have been plans to change that for a while (e.g. your own https://bugzilla.mozilla.org/show_bug.cgi?id=636145, mentioned by Alexandre in another post here).

Multiple "instances" of the same code (page-mods, unit-test)

I guess we could create multiple modules instances, but as far as I know we don't do it.

We do in unit-tests, as I mentioned, and it's worth keeping in mind:
http://www.google.com/codesearch#search/&q=require%5C%28%22cuddlefish%22%5C%29%20package:git://github.com/mozilla/addon-sdk.git&type=cs
http://www.google.com/codesearch#search/&q=makesandboxedloader%20package:git://github.com/mozilla/addon-sdk.git&type=cs

Page-mods are essentially multiple copies of the same code running in different context. jsm modules cannot be used for that, while page-mods would most benefit from the caching, as discussed below.

 Unloadable

Do you suggest that sandboxes may be destroyed in some way ? 

Sandboxes participate in GC (that's what makes uninstall-without-restart work), while Cu.import modules are rooted and the knob to unload them only appeared in a recent Firefox version.

Many extra globals: atob, btoa, File, __LOCATION__, __URI__, EXPORTED_SYMBOLS

Actually having atob / boat is awesome, as we won't have to use hidden window for that :D (I did not knew they were there)!!!

I agree that those globals are inconvenient, but they can be easily shadowed:

https://github.com/mozilla/addon-sdk/pull/212/files#L1R207

Arguably, with the module system you want to have a clean slate with a bunch of modules you can import to get the necessary functionality. That reduces the amount of magic (c.f. your comment about atob), so it's easier to learn and understand. 

As far as I know, the main problem with sandboxes [...] no caching of compiled code.

I don't think compiled code cacheing is relevant, as module from the same url is never executed more than once.

...per startup. It's very much relevant. otherwise there would be no startup cache for Firefox code. See my earlier post http://groups.google.com/group/mozilla-labs-jetpack/browse_thread/thread/6d0b8401ba698f36/842af69769b09bcc , where I cite 25% loss (200ms on my old laptop) in startup performance for a simple add-on, of which 25% is spent compiling and evaluating the JS code. (This doesn't allow to estimate the exact wins from caching, but shows there's a room to improve.)

Also, please keep the page-mods in mind. Their code is re-evaluated for *every page load*.

We cache module exports so that second require('foo') will return exports from the cache. Also note sure you're aware, SDK modules are bundled with each add-on and are mapped to a diff resource URIs, which means that exact same sdk module is executed per addon.

Thanks, I know that.

Is there anything I'm missing? Does anyone know the performance/memory characteristics of Sandbox vs Cu.import modules?

Main reason I've started with this loader is to simplify complexity of the current loader and offloading as much to the platform as possible. So ideas is to replace all of this:

I'm very much on board with the simplification of the loader, since I had to read through that code a few times! Thanks for doing that, by the way!

It's just that seeing someone actually try changing sandboxes to jsm modules made me think about their pros and cons.

Nickolay

[Nickolay Ponomarev]

Hi Alexandre,

On Wed, Jul 20, 2011 at 8:09 PM, Alexandre poirot <poiro...@gmail.com> wrote:

From my point of view, I think that the main drawback of Cu.import is the lack of control of the JS context. We can't set our custom principal, nor JS version.

Exactly. Also Cu.import forces its module system on us, when we have our own CommonJS one.
From the API side, the Sandbox is more of a primitive building block (creating separate contexts for JS code), while Cu.import is a more high-level thing: an implementation of (non-CommonJS) modules.

So it is obvious that we have to improve our performances [...] Then, from what I know, we have only two options: Cu.import or loadSubScript.

(Or extending the platform to support the new method on the sandbox. Since it's an incremental improvement, we can use it immediately on versions that support it.)

[...]

Finally, I would say that the priority is to simplify our codebase with proposal like Irakli's loader, then improving performance is going to be wayyyy simplier!

I very much agree with the rest of your message, particularly with this last sentence :)

Nickolay

[Irakli Gozalishvili]

On Wednesday, 2011-07-20 at 21:35 , Nickolay Ponomarev wrote:

Hi,

Thanks for your reply! I'll reply both to you and (indirectly) Dave here, since you raise similar points.

On Wed, Jul 20, 2011 at 4:52 AM, Irakli Gozalishvili <rfo...@gmail.com> wrote:

On Tuesday, 2011-07-19 at 22:51 , Nickolay Ponomarev wrote:

Here's a brief comparison of capabilities I put together: https://wiki.mozilla.org/User:Asqueella/Sandboxes

Here is few comments on that:

Run code with non-system principal

All jetpack modules run with system principal anyway.

 

At this time yes, but there have been plans to change that for a while (e.g. your own https://bugzilla.mozilla.org/show_bug.cgi?id=636145, mentioned by Alexandre in another post here).

Multiple "instances" of the same code (page-mods, unit-test)

I guess we could create multiple modules instances, but as far as I know we don't do it.

We do in unit-tests, as I mentioned, and it's worth keeping in mind:
http://www.google.com/codesearch#search/&q=require%5C%28%22cuddlefish%22%5C%29%20package:git://github.com/mozilla/addon-sdk.git&type=cs
http://www.google.com/codesearch#search/&q=makesandboxedloader%20package:git://github.com/mozilla/addon-sdk.git&type=cs

Those test loader itself, except the second one which is used by test runner, also we want to change that one as at the moment test runner takes a diff code path from when it's used as add-on.   

 

Page-mods are essentially multiple copies of the same code running in different context. jsm modules cannot be used for that, while page-mods would most benefit from the caching, as discussed below.

I guess you refer to the content scripts, that are used in page mods and some other APIs for interaction with content. If so it's not related to the module loader and we will continue using sandboxes there.

 

 Unloadable

Do you suggest that sandboxes may be destroyed in some way ? 

Sandboxes participate in GC (that's what makes uninstall-without-restart work), while Cu.import modules are rooted and the knob to unload them only appeared in a recent Firefox version.

 

I see, that's definitely something we should consider, while making this decision!!

 

Many extra globals: atob, btoa, File, __LOCATION__, __URI__, EXPORTED_SYMBOLS

Actually having atob / boat is awesome, as we won't have to use hidden window for that :D (I did not knew they were there)!!!

I agree that those globals are inconvenient, but they can be easily shadowed:

https://github.com/mozilla/addon-sdk/pull/212/files#L1R207

Arguably, with the module system you want to have a clean slate with a bunch of modules you can import to get the necessary functionality. That reduces the amount of magic (c.f. your comment about atob), so it's easier to learn and understand.

Agreed! 

 

As far as I know, the main problem with sandboxes [...] no caching of compiled code.

I don't think compiled code cacheing is relevant, as module from the same url is never executed more than once.

...per startup. It's very much relevant. otherwise there would be no startup cache for Firefox code. See my earlier post http://groups.google.com/group/mozilla-labs-jetpack/browse_thread/thread/6d0b8401ba698f36/842af69769b09bcc , where I cite 25% loss (200ms on my old laptop) in startup performance for a simple add-on, of which 25% is spent compiling and evaluating the JS code. (This doesn't allow to estimate the exact wins from caching, but shows there's a room to improve.)

I remember that thread, actually that was a trigger that made me invest into JSM :) Sorry, I still don't understand, is compiled code cached in some way that it is reused across multiple startups ? If that's not the case how is it going to help ?

 

Also, please keep the page-mods in mind. Their code is re-evaluated for *every page load*.

As mentioned above, page-mods are diff story, they are not handled by module loader. Probably it would make sense to have separate thread for this one.  

 

We cache module exports so that second require('foo') will return exports from the cache. Also note sure you're aware, SDK modules are bundled with each add-on and are mapped to a diff resource URIs, which means that exact same sdk module is executed per addon.

Thanks, I know that.

Is there anything I'm missing? Does anyone know the performance/memory characteristics of Sandbox vs Cu.import modules?

Main reason I've started with this loader is to simplify complexity of the current loader and offloading as much to the platform as possible. So ideas is to replace all of this:

I'm very much on board with the simplification of the loader, since I had to read through that code a few times! Thanks for doing that, by the way!

It's just that seeing someone actually try changing sandboxes to jsm modules made me think about their pros and cons.

I'm very happy you started this thread actually!! As I already mentioned, your other thread made me think that jsm was a better option :)

BTW I mentioned in the bug Alex mentioned, do you think using function wrappers is a better option ??  

[Alexandre poirot]

Actually, we rely on the possibility to instanciate the same module multiple times in tests.
Thanks to test.makeSandboxedLoader(), we are able to intanciate a new Cuddlefish.
It is important to test:
 - correct module unload:
     https://github.com/mozilla/addon-sdk/blob/master/packages/addon-kit/tests/test-widget.js#L32
 - how behave multiple instances of the same module:
     https://github.com/mozilla/addon-sdk/blob/master/packages/addon-kit/tests/test-widget.js#L115

Then Irakli, you are right, page-mod is another subject and has nothing to do with your loader experiment,
but still, Nickolay is right too by bringing them into this particular topic which is about "import vs loadSubScript". So that advantage of StartupCache is even more meaningfull for content scripts!

I think that we can easily agree that we can't use Cu.import as:
 - Cu.unload won't be available until FF7 and module unload is a mandatory feature,
 - it is going to be harder to offer multiple same module instance with Cu.import.

Then, we have loadSubScript that allow us to have:
 - better control of principal (give space for improvement in that subject),
 - multiple instance easily,
 - better performance by avoiding reading modules in JS (compared to current implementation)
But StartupCache won't be available in loadSubScript before Firefox 8:
  https://bugzilla.mozilla.org/show_bug.cgi?id=648125

I think that we can setup a non-system-principal when using loadSubScript.
The following script throw a security exception on NS_ASSERT execution,
because "http://mozilla.org" principal can't have use Components.utils,
that is used in NS_ASSERT.
This script can be executed in js console:
var obj = {};
var loader = Components.classes["@mozilla.org/moz/jssubscript-loader;1"]                        .getService(Components.interfaces.mozIJSSubScriptLoader);
var s = Components.utils.Sandbox("http://mozilla.org");
loader.loadSubScript("chrome://global/content/globalOverlay.js", s);
s.NS_ASSERT();

I'll try to provide a patch for page-mod in order to use loadSubScript for contentScriptFile.
Nickolay: do you still have performance profiling patch running on current master?
I prefer to push such improvement while being sure that it really improves things significantly :)

2011/7/21 Irakli Gozalishvili <rfo...@gmail.com>

[Brian Warner]

I'm late to this party, but had a couple of small thoughts:

On 7/19/11 9:51 PM, Dave Townsend wrote:
>
> I think in general you want the system principal right now. Presumably
> you could also load a couple of small parts of code into a sandbox
> somehow should you require a non-system principal for a specific set
> of code.

As Alexandre has been working on in 636145, we'd really like for only
the lowest-level require("chrome")-using modules to have the system
principal, and for the rest to be denied XPCOM access. That's critical
to allow security reviews of non-chrome modules to rely upon their
dependencies list (the module-graph "manifest") as providing an upper
bound on the module's authority.

> I actually think that (with proper version handling) allowing multiple
> add-ons to essentially access the same SDK modules would be a great
> way to save the performance hit per add-on. Perhaps that doesn't make
> a difference once we're in an e10s world though.

It should still be an option, though: putting multiple add-ons in the
same process ought to save us memory on the shared components.

There's a security angle here too. Sharing module instances enables
communication: if your add-on and my add-on both do
require("something"), and something.js has set/get methods, we can send
information and objects through it. The common module must be a willing
participant (it may be possible to audit the code to prove it posesses a
property named "DeepFrozen", in which case it cannot act as a
communication channel, but that's a much bigger project[1]).

From a what-authority-does-require()-grant-you point of view, importing
a module that doesn't itself include any other dependencies shouldn't
give you any new powers: reviewers should be able to treat that just
like manually pasting in the contents of the included module, so no
difference between:

foo.js:
require("bar").blah()
bar.js:
exports.blah = function() {print("hello");}

and:

foo.js:
tmp.blah = function() {print("hello");}
blah()

But shared instances gives more power than that: it lets you collude
with other users. If bar.js looks like:

var shared;
exports.set = function(value) {shared=value;}
exports.get = function() {return shared;}

then there's a big difference between bar.js being interpolated/pasted
in, versus it living in a single shared instance.

That said, jetpack currently uses shared instances of modules within a
single add-on, because that's absolutely necessary to make most of our
APIs work, and we just all remember that shared nodes in the module
graph enable collusion.

cheers,
-Brian

[1]: http://citeseer.ist.psu.edu/viewdoc/summary?doi=10.1.1.123.9021
http://wiki.erights.org/wiki/DeepFrozen