2,900 instances of "NS_ENSURE_TRUE(IsSandbox(sb)) failed" emitted from js/xpconnect/src/XPCComponents.cpp during linux64 debug testing

RESOLVED FIXED in Firefox 50

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: erahm, Assigned: kmag)

Tracking

(Blocks 1 bug)

Trunk
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed, firefox51 fixed, firefox52 fixed)

Details

Attachments

(2 attachments)

> 2926 WARNING: NS_ENSURE_TRUE(IsSandbox(sb)) failed: file js/xpconnect/src/XPCComponents.cpp, line 3024

This warning [1] shows up in the following test suites:

>   2757 - desktop-test-linux64/debug-mochitest-jetpack JP
>    114 - desktop-test-linux64/debug-mochitest-devtools-chrome-9 dt9
>     55 - desktop-test-linux64/debug-mochitest-chrome-3 c3

It shows up in 41 tests. A few of the most prevalent:

>    198 -        page-mod-debugger-post/main.testDebugger
>    110 -        devtools/client/debugger/test/mochitest/browser_dbg_listaddons.js
>    106 -        places/main.testVisitCount
>    100 -        content-permissions/main.testCrossDomainXHR
>    100 -        content-script-messages-latency/main.testContentScriptLatencyRegression
>     99 -        test-require/main.testSDKRequire
>     98 -        simple-prefs/main.testOptionsType
>     98 -        simple-prefs-l10n/main.testAOMLocalization
>     94 -        chrome/main.testChromeSkin
>     93 -        tab-close-on-startup/main.testNoTabCloseOnStartup

[1] https://hg.mozilla.org/mozilla-central/annotate/f9f3cc95d728/js/xpconnect/src/XPCComponents.cpp#l3024
That seems to suggest that the jetpack code passes something that's not a sandbox to nukeSandbox, which is probably a bug.
Regression from bug 1309351?
Assignee

Comment 3

3 years ago
This is mainly just an issue with the legacy bootstrap code that's still used by the Jetpack tests. I guess we should just check whether the module has its own sandbox from JS rather than catching the exception that nukeSandbox throws.
Assignee: nobody → kmaglione+bmo
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Comment 6

3 years ago
Let me know if you want me to redirect this review, but the changes are pretty simple, and the code doesn't really have an owner at this point.
Reporter

Comment 7

3 years ago
mozreview-review
Comment on attachment 8805257 [details]
Bug 1313440: Fix NS_ENSURE_TRUE(isSandbox) warnings in test code.

https://reviewboard.mozilla.org/r/89036/#review88182

::: addon-sdk/source/app-extension/bootstrap.js:301
(Diff revision 1)
>    scriptLoader.loadSubScript(uri, sandbox, 'UTF-8');
>    return sandbox;
>  }
>  
>  function unloadSandbox(sandbox) {
> -  if ("nukeSandbox" in Cu) {
> +  if (Cu.getClassName(sandbox, true) == "Sandbox")

I guess nukeSandbox is always a thing at this point? Can it throw or is that not a concern?
Reporter

Comment 8

3 years ago
mozreview-review
Comment on attachment 8805257 [details]
Bug 1313440: Fix NS_ENSURE_TRUE(isSandbox) warnings in test code.

https://reviewboard.mozilla.org/r/89036/#review88184
Attachment #8805257 - Flags: review?(erahm) → review+
Reporter

Comment 9

3 years ago
mozreview-review
Comment on attachment 8805258 [details]
Fix NS_ENSURE_TRUE(isSandbox) warnings in JPM bootstrap.

https://reviewboard.mozilla.org/r/89038/#review88186

Ah right it shouldn't throw because we've verified it's a sandbox. If you can to a try run with the JP tests I can verify the warnings are gone.
Attachment #8805258 - Flags: review?(erahm) → review+
Assignee

Comment 10

3 years ago
mozreview-review-reply
Comment on attachment 8805257 [details]
Bug 1313440: Fix NS_ENSURE_TRUE(isSandbox) warnings in test code.

https://reviewboard.mozilla.org/r/89036/#review88182

> I guess nukeSandbox is always a thing at this point? Can it throw or is that not a concern?

Yeah. This is ancient code from the time when the SDK was bundled with every add-on that used it, rather than included in Firefox.
Looks like we're good!

> 4 WARNING: NS_ENSURE_TRUE(IsSandbox(sb)) failed: file js/xpconnect/src/XPCComponents.cpp, line 3024

This warning [1] shows up in the following test suites:

>      4 - desktop-test-linux64/debug-mochitest-devtools-chrome-9 dt9

It shows up in 4 tests. A few of the most prevalent:

>      1 -        devtools/client/debugger/test/mochitest/browser_dbg_addon-workers-dbg-enabled.js
>      1 -        devtools/client/debugger/test/mochitest/browser_dbg_addon-panels.js
>      1 -        devtools/client/debugger/test/mochitest/browser_dbg_addonactor.js
>      1 -        devtools/client/debugger/test/mochitest/browser_dbg_addon-sources.js

[1] https://hg.mozilla.org/try/annotate/30dc9a187102/js/xpconnect/src/XPCComponents.cpp#l3024
Assignee

Comment 13

3 years ago
Ick. It looks like those tests bundle pre-built XPIs with ancient versions of the old style bootstrap.js loader. I'm not going to worry about them for now. They should be updated to use the new bootstrap code at some point, though.
Assignee

Comment 15

3 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/353c47fc71ccff8b243c66df027350fe30dbce61
https://hg.mozilla.org/releases/mozilla-beta/rev/2eafbd4df272f9edb5160377530512add9370378

Only uplifting the test-only parts. The bootstrap.js that's part of the build is used by very few tests, so it shouldn't be a major issue.

Comment 16

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fb2d15e4f33d
https://hg.mozilla.org/mozilla-central/rev/94d5d14da6ba
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.