1302441 - Hit MOZ_ASSERT(preconditionForWriteBarrierPost(owner, kind, slot, target)) from DebuggerFrame.environment getter

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Alexandre Poirot [:ochameau]]

Looks like a race or something as it only happens under special setup.

But if you apply attachment 8788396 [details] and run:
./mach mochitest devtools/client/webconsole/test/ --disable-e10s

You will hit this assertion when running this test:
devtools/client/webconsole/test/browser_console_optimized_out_vars.js
https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/test/browser_console_optimized_out_vars.js?q=file%3Abrowser_console_optimized_out_vars.js&redirect_type=single

It asserts from this JS line:
https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/frame.js#64
    if (this.frame.environment) {
With this stack:
FrameActor<.form@resource://devtools/server/actors/frame.js:65:10
ThreadActor<._paused@resource://devtools/server/actors/script.js:1562:22
ThreadActor<._pauseAndRespond@resource://devtools/server/actors/script.js:699:20
BreakpointActor<.hit@resource://devtools/server/actors/breakpoint.js:172:12
outer/<@http://example.com/browser/devtools/client/webconsole/test/test-closure-optimized-out.html:18:15
clickHandler@http://example.com/browser/devtools/client/webconsole/test/test-closure-optimized-out.html:23:11
@chrome://mochikit/content/tests/BrowserTestUtils/content-task.js line 52 > eval:6:7
TaskImpl_run@resource://gre/modules/Task.jsm:319:40
TaskImpl@resource://gre/modules/Task.jsm:280:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:254:14
Task_spawn@resource://gre/modules/Task.jsm:168:12
@chrome://mochikit/content/tests/BrowserTestUtils/content-task.js:54:5


This test asserts some properties of "optimized out" variable against the debugger. And this patch add calls to forceGC/forceCC at test end.
Note that it doesn't reproduce if you run just the given test, you have to run the whole folder. I imagine we have to populate the GC to make it crash.


Here is the c++ stack:
#0  0x00007fffe98117c2 in js::HeapSlot::post (target=..., slot=1, kind=js::HeapSlot::Slot, owner=0x7fffdcb1b438, this=0x7fffdcb1b460)
    at /mnt/desktop/gecko/js/src/gc/Barrier.h:703
#1  0x00007fffec76e197 in js::HeapSlot::post (this=0x7fffdcb1b460, owner=0x7fffdcb1b438, kind=js::HeapSlot::Slot, slot=1, target=...)
    at /mnt/desktop/gecko/obj-firefox/dist/include/js/Value.h:807
#2  0x00007fffecc05ce5 in init (v=..., slot=1, kind=js::HeapSlot::Slot, owner=<optimized out>, this=<optimized out>) at /mnt/desktop/gecko/js/src/gc/Barrier.h:680
#3  initFixedSlot (value=..., slot=1, this=<optimized out>) at /mnt/desktop/gecko/js/src/vm/NativeObject.h:927
#4  js::CallObject::createHollowForDebug (cx=0x7fffe0dc9000, callee=..., callee@entry=...) at /mnt/desktop/gecko/js/src/vm/EnvironmentObject.cpp:290
#5  0x00007fffecc06398 in GetDebugEnvironmentForMissing (ei=..., cx=0x7fffe0dc9000) at /mnt/desktop/gecko/js/src/vm/EnvironmentObject.cpp:2878
#6  GetDebugEnvironment (cx=cx@entry=0x7fffe0dc9000, ei=...) at /mnt/desktop/gecko/js/src/vm/EnvironmentObject.cpp:2937
#7  0x00007fffecc06a57 in js::GetDebugEnvironmentForFrame (cx=0x7fffe0dc9000, frame=..., pc=pc@entry=0x7fffb0e49500 "\210")
    at /mnt/desktop/gecko/js/src/vm/EnvironmentObject.cpp:2966
#8  0x00007fffecc1b541 in js::DebuggerFrame::getEnvironment (cx=cx@entry=0x7fffe0dc9000, frame=..., frame@entry=..., result=..., result@entry=...)
    at /mnt/desktop/gecko/js/src/vm/Debugger.cpp:7284
#9  0x00007fffecc1b6d3 in js::DebuggerFrame::environmentGetter (cx=cx@entry=0x7fffe0dc9000, argc=<optimized out>, vp=<optimized out>)
    at /mnt/desktop/gecko/js/src/vm/Debugger.cpp:7803
#10 0x00007fffecc938c5 in js::CallJSNative (cx=0x7fffe0dc9000, native=0x7fffecc1b620 <js::DebuggerFrame::environmentGetter(JSContext*, unsigned int, JS::Value*)>, args=...)
    at /mnt/desktop/gecko/js/src/jscntxtinlines.h:235
#11 0x00007fffecc8a24b in js::InternalCallOrConstruct (cx=cx@entry=0x7fffe0dc9000, args=..., construct=construct@entry=js::NO_CONSTRUCT)
    at /mnt/desktop/gecko/js/src/vm/Interpreter.cpp:454
#12 0x00007fffecc8a6ab in InternalCall (cx=cx@entry=0x7fffe0dc9000, args=...) at /mnt/desktop/gecko/js/src/vm/Interpreter.cpp:499
#13 0x00007fffecc8a81a in js::Call (cx=cx@entry=0x7fffe0dc9000, fval=..., fval@entry=..., thisv=..., thisv@entry=..., args=..., rval=..., rval@entry=...)
    at /mnt/desktop/gecko/js/src/vm/Interpreter.cpp:518
#14 0x00007fffecc8a917 in js::CallGetter (cx=0x7fffe0dc9000, thisv=..., thisv@entry=..., getter=getter@entry=..., rval=rval@entry=...)
    at /mnt/desktop/gecko/js/src/vm/Interpreter.cpp:632
#15 0x00007fffecc8acb7 in CallGetter (vp=..., shape=..., receiver=..., obj=..., cx=0x7fffe0dc9000) at /mnt/desktop/gecko/js/src/vm/NativeObject.cpp:1757
#16 GetExistingProperty<(js::AllowGC)1> (cx=0x7fffe0dc9000, receiver=..., obj=..., shape=..., vp=...) at /mnt/desktop/gecko/js/src/vm/NativeObject.cpp:1809
#17 0x00007fffecc8b6bd in NativeGetPropertyInline<(js::AllowGC)1> (cx=0x7fffe0dc9000, obj=..., receiver=..., id=..., nameLookup=nameLookup@entry=NotNameLookup, vp=...)
    at /mnt/desktop/gecko/js/src/vm/NativeObject.cpp:2032
#18 0x00007fffecc8bd50 in js::NativeGetProperty (cx=<optimized out>, obj=..., obj@entry=..., receiver=..., receiver@entry=..., id=..., id@entry=..., vp=..., vp@entry=...)
    at /mnt/desktop/gecko/js/src/vm/NativeObject.cpp:2066
#19 0x00007fffecbbb478 in GetProperty (vp=..., id=..., receiver=..., obj=..., cx=<optimized out>) at /mnt/desktop/gecko/js/src/vm/NativeObject.h:1491
#20 js::Wrapper::get (this=this@entry=0x7fffeee08e60 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0x7fffe0dc9000, proxy=..., proxy@entry=..., 
    receiver=receiver@entry=..., id=id@entry=..., vp=vp@entry=...) at /mnt/desktop/gecko/js/src/proxy/Wrapper.cpp:143
#21 0x00007fffecb7c693 in js::CrossCompartmentWrapper::get (this=0x7fffeee08e60 <js::CrossCompartmentWrapper::singleton>, cx=0x7fffe0dc9000, wrapper=..., receiver=..., 
    id=..., vp=...) at /mnt/desktop/gecko/js/src/proxy/CrossCompartmentWrapper.cpp:209
#22 0x00007fffecb77d3a in js::Proxy::get (cx=0x7fffe0dc9000, proxy=..., receiver_=..., id=..., vp=...) at /mnt/desktop/gecko/js/src/proxy/Proxy.cpp:310
#23 0x00007fffecc8cc83 in GetProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=0x7fffe0dc9000) at /mnt/desktop/gecko/js/src/jsobj.h:846
#24 js::GetProperty (cx=0x7fffe0dc9000, v=..., name=..., vp=...) at /mnt/desktop/gecko/js/src/vm/Interpreter.cpp:4245
#25 0x00007fffecc7d31f in GetPropertyOperation (vp=..., lval=..., pc=<optimized out>, script=..., fp=<optimized out>, cx=0x7ffff7fd7780)
    at /mnt/desktop/gecko/js/src/vm/Interpreter.cpp:190

Comment 1

[Alexandre Poirot [:ochameau]]

Eddy, Jim, any idea? Any one else to ping? Is Debtools:debugger the right component for this assertion?

Comment 2

[Jim Blandy :jimb]

Yes, it's the right component. It's almost certainly a Debugger bug, if it's not random memory trashing.

I'm leaving the needinfo mark until I can see if I can reproduce.

Comment 3

[Alexandre Poirot [:ochameau]]

If you don't, I do, so do not hesitate to guide me.

The STR should be to apply attachment 8788396 [details] on top of a recent tip.
But note that this patch comes from bug 1281341, that is about to land with a slightly different patch which workaround this assertion...

Then, just run this and wait for a while as it runs tons of tests before asserting:
./mach mochitest devtools/client/webconsole/test/ --disable-e10s
It always repro for me on linux.

Comment 4

[Jim Blandy :jimb]

I let this run for a long time but it didn't crash. I have to go home now, but I'll set it running there and try again.

Comment 5

[Eddy Bruel [:ejpbruel]]

(In reply to Jim Blandy :jimb from comment #4)
> I let this run for a long time but it didn't crash. I have to go home now,
> but I'll set it running there and try again.

Could this be a regression from http://hg.mozilla.org/mozilla-central/rev/763679990ba8 ?

Comment 6

[Jim Blandy :jimb]

If it is that changeset, I'd be surprised. I looked the patch over and it still seems fine to me.

Comment 7

[Jim Blandy :jimb]

darnit

Comment 8

[Jim Blandy :jimb]

Okay, just to be sure: the patch I'm supposed to apply is revision 4 here:

https://reviewboard.mozilla.org/r/76896/diff/4/

which adds a single call to requestLongerTimeout. Is that correct?

Comment 9

[Alexandre Poirot [:ochameau]]

No, this one (attachment links refer to last mozreview changeset, but this changed over time to workaround this crash):
  https://reviewboard.mozilla.org/r/76896/diff/2/
The important part is the call to forceCollections() from registerCleanupFunction callback in shared-head.js

You also need to m-c checkout before we landed bug 1281341 to prevent conflict when applying this and also prevent having the workaround preventing this crash... (you may also just revert but I don't know what will happen).

Comment 10

[Ryan VanderMeulen [:RyanVM]]

This is apparently the same as bug 1287047, which is frequent and is known bustage from the front-end rewrite. Can you please help investigate, Shu?

Comment 11

[Shu-yu Guo [:shu]]

Redirecting NI to jimb, who was looking at bug 1287047. I have a quick fix in mind where we exposeToActiveJS the gray callee, but jimb is investigating if that fix is correct or it is papering over a bigger problem.

Comment 12

[Jim Blandy :jimb]

I was trying to reproduce this, and dropped it on the floor. Sorry about that; I'm picking this up again.

What I'm looking at now is 9e934ce03669 pulled from https://reviewboard-hg.mozilla.org/gecko, relative to its original parent 22565d35ed35, which is an ancestor of central.

Comment 13

[Jim Blandy :jimb]

Attachment: Break assertion down into smaller assertions.

Comment 14

[Jim Blandy :jimb]

I'm not able to reproduce this bug. Alex, if you're still able to reproduce it, could you apply the attached patch, which should refine the assertions, so we can be sure which sort of error we're encountering?

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a762f563290
Break assertion down into smaller assertions. r=shu

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2a762f563290

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:jlast, maybe it's time to close this bug?

Comment 18

[Jason Laster [:jlast]]

redirecting to jim.

Comment 19

[Jim Blandy :jimb]

Oh, I dropped this. I'll resuscitate it and get it landed. Sorry.

Comment 20

[Jim Blandy :jimb]

Okay, this doesn't need any further work, it can be closed. The change to the assertion I proposed has landed; and bug 1287047 landed and seemed to fix the problem.