The default bug view has changed. See this FAQ.

GC Hazard with new Xray Expando Architecture (Jetpack compartment mismatch)

RESOLVED FIXED in Firefox 15

Status

()

Core
XPConnect
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: KWierso, Unassigned)

Tracking

unspecified
mozilla16
x86_64
Windows 8
Points:
---

Firefox Tracking Flags

(firefox15 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Starting with this push ( https://tbpl.mozilla.org/?noignore=1&tree=Mozilla-Inbound&rev=828da55601e7 )on inbound, all Jetpack test runs on debug builds are failing with a timeout error.

Assertion failure: (obj2)->compartment()->isCollecting(), at ../../../js/src/gc/Marking.cpp:1142
Traceback (most recent call last):
  File "bin/cfx", line 33, in <module>
    cuddlefish.run()
  File "/home/cltbld/talos-slave/test/build/addon-sdk-07137b290b6d/python-lib/cuddlefish/__init__.py", line 517, in run
    test_all(env_root, defaults=options.__dict__)
  File "/home/cltbld/talos-slave/test/build/addon-sdk-07137b290b6d/python-lib/cuddlefish/__init__.py", line 345, in test_all
    test_all_packages(env_root, defaults)
  File "/home/cltbld/talos-slave/test/build/addon-sdk-07137b290b6d/python-lib/cuddlefish/__init__.py", line 408, in test_all_packages
    env_root=env_root)
  File "/home/cltbld/talos-slave/test/build/addon-sdk-07137b290b6d/python-lib/cuddlefish/__init__.py", line 786, in run
    mobile_app_name=options.mobile_app_name)
  File "/home/cltbld/talos-slave/test/build/addon-sdk-07137b290b6d/python-lib/cuddlefish/runner.py", line 675, in run_app
    OUTPUT_TIMEOUT)
Exception: Test output exceeded timeout (60s).


There have been two pushes to inbound since that one, and they both have all debug builds failing:
https://tbpl.mozilla.org/?noignore=1&tree=Mozilla-Inbound&jobname=jetpack&rev=61fd66629c4f
https://tbpl.mozilla.org/?noignore=1&tree=Mozilla-Inbound&jobname=jetpack&rev=4c34e1a4cbb5
How do I run these tests myself? Tinderbox is running a script called run_jetpack.py, but I don't have that in my tree.
(Reporter)

Comment 2

5 years ago
(In reply to Bill McCloskey (:billm) from comment #1)
> How do I run these tests myself? Tinderbox is running a script called
> run_jetpack.py, but I don't have that in my tree.

You can download the SDK source from here: http://hg.mozilla.org/projects/addon-sdk/archive/07137b290b6d.tar.bz2 and unpack it somewhere. 

Go into the unpacked directory and then do the following on the command line (assuming you're on Linux or Mac):
source bin/activate
cfx testall


This isn't exactly what Tinderbox does, but I think it'd be about the same end result.

When you're done you can just type | deactivate | to get rid of the extra stuff added to the command line.
(Reporter)

Comment 3

5 years ago
Ooh, you'd probably need to do "cfx testall -b /path/to/debug/binary" to get it to actually use the debug build.
Bobby, could you take a look at this? It looks like a regression from bug 758415.

When I run the test, there's XrayExpandoObject pointing at a Sandbox object in a different compartment. It looks like it's via the JSSLOT_EXPANDO_EXCLUSIVE_GLOBAL slot. I assume this is intentional, but nevertheless it's a problem for the GC.

If you can guarantee that the value in the JSSLOT_EXPANDO_EXCLUSIVE_GLOBAL slot will outlive the XrayExpandoObject, then you could put a PrivateValue rather than an ObjectValue in the slot. You just have to worry about what might happen if the old exclusive global is freed and something else is allocated in its place--then you might get false matches.

The alternative is to wrap the global before sticking it in there.
Created attachment 632229 [details] [diff] [review]
Part 1 - Pass cx around in more places. v1
Attachment #632229 - Flags: review?(mrbkap)
Created attachment 632230 [details] [diff] [review]
Part 2 - Fix up compartment situation for expando objects. v1
Attachment #632230 - Flags: review?(mrbkap)
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=9ca60839bec3

Sigh, sorry for making such a dumb mistake, bill.

I can't seem to reproduce this crash locally. Can you double-check that the patches here fix the problem?
(assuming you can easily reproduce it yourself. We can also check the results of the try push with jetpack unhidden: https://tbpl.mozilla.org/?tree=Try&rev=9ca60839bec3&noignore=1 )
MOZ_ASSERT_IF is nice, btw.
The JP tests are green, so I don't think there's any need to test locally. Thanks, Bobby.
(Reporter)

Updated

5 years ago
Priority: -- → P1
It seems likely that this is causing bug 763818. Can you get to this soon, Blake?

Updated

5 years ago
Attachment #632229 - Flags: review?(mrbkap) → review+
Comment on attachment 632230 [details] [diff] [review]
Part 2 - Fix up compartment situation for expando objects. v1

Review of attachment 632230 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/wrappers/XrayWrapper.cpp
@@ +134,5 @@
> +    // work needs to happen there.
> +    JSAutoEnterCompartment ac;
> +    if (!ac.enter(cx, target) ||
> +        !JS_WrapObject(cx, &exclusiveGlobal))
> +        return NULL;

Nit: need braces around the consequent here.
Attachment #632230 - Flags: review?(mrbkap) → review+
Pushed to m-i:

http://hg.mozilla.org/integration/mozilla-inbound/rev/7b4ee17ebace
http://hg.mozilla.org/integration/mozilla-inbound/rev/662e6d9d368d
Assignee: nobody → bobbyholley+bmo
Summary: Jetpack tests timing out on all debug builds after triggering a compartment assertion failure → GC Hazard with new Xray Expando Architecture (Jetpack compartment mismatch)
https://hg.mozilla.org/mozilla-central/rev/662e6d9d368d
https://hg.mozilla.org/mozilla-central/rev/7b4ee17ebace
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 632230 [details] [diff] [review]
Part 2 - Fix up compartment situation for expando objects. v1

Review of attachment 632230 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/wrappers/XrayWrapper.cpp
@@ +212,5 @@
>  {
> +    // Expando objects live in the target compartment.
> +    JSAutoEnterCompartment ac;
> +    if (!ac.enter(cx, target))
> +        return false;

false ain't no JSObject*...
Assignee: bobbyholley+bmo → nobody
Component: General → XPConnect
Product: Add-on SDK → Core
QA Contact: general → xpconnect
Comment on attachment 632229 [details] [diff] [review]
Part 1 - Pass cx around in more places. v1

Nominating per bug 758415 comment 56.
Attachment #632229 - Flags: approval-mozilla-aurora?
Attachment #632230 - Flags: approval-mozilla-aurora?
Comment on attachment 632229 [details] [diff] [review]
Part 1 - Pass cx around in more places. v1

[Triage Comment]
In support of a tracked bug, approved for Aurora 15.
Attachment #632229 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #632230 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 763818

Comment 19

5 years ago
Landed in Aurora 15 along time ago:
http://hg.mozilla.org/releases/mozilla-aurora/rev/e5f9e8f2816f
http://hg.mozilla.org/releases/mozilla-aurora/rev/c5fb78e2f25d
status-firefox15: --- → fixed
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.