729507 - Crash in JSObject::finish

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Marcia Knous [:marcia]]

Seen while looking at the explosive report. This signatures seems to be new to 11.0B3. https://crash-stats.mozilla.com/report/list?signature=je_free%20|%20mozglue.dll@0x34cf. Moderate volume crash that seems to be more heavily distributed towards Win XP.

https://crash-stats.mozilla.com/report/index/5c798c6b-f06c-4e98-8869-6116a2120222

Frame 	Module 	Signature [Expand] 	Source
0 	mozglue.dll 	je_free 	memory/jemalloc/jemalloc.c:6580
1 	mozglue.dll 	mozglue.dll@0x34cf 	
2 	mozjs.dll 	JSObject::finish 	js/src/jsobjinlines.h:1107
3 	mozjs.dll 	js::gc::Arena::finalize<JSObject> 	js/src/jsgc.cpp:311
4 	mozjs.dll 	js::gc::FinalizeTypedArenas<JSObject> 	js/src/jsgc.cpp:358
5 	mozjs.dll 	js::gc::FinalizeArenas 	js/src/jsgc.cpp:398
6 	mozjs.dll 	js::gc::ArenaLists::backgroundFinalize 	js/src/jsgc.cpp:1538
7 	mozjs.dll 	js::GCHelperThread::doSweep 	js/src/jsgc.cpp:2396
8 	mozjs.dll 	js::GCHelperThread::threadLoop 	js/src/jsgc.cpp:2276
9 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:426
10 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:122
11 	msvcr80.dll 	_callthreadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c:348
12 	msvcr80.dll 	_threadstartex 	f:\\dd\\vctools\\crt_bld\\self_x86\\crt\\src\\threadex.c:326
13 	kernel32.dll 	BaseThreadStart 

Some manual correlations:

je_free | mozglue.dll@0x34cf|EXCEPTION_ACCESS_VIOLATION_READ (58 crashes)
     21% (12/58) vs.   1% (160/29707) [e-mail]
     21% (12/58) vs.   2% (477/29707) [e-mail] (IDM CC, https://addons.mozilla.org/addon/6973)
     21% (12/58) vs.   2% (652/29707) [e-mail]
     90% (52/58) vs.  80% (23676/29707) [e-mail] (Mozilla Labs - Test Pilot, https://addons.mozilla.org/addon/13661)
    100% (58/58) vs.  94% (28023/29707) {972ce4c6-7e08-4474-a285-3208198ce6fd} (Default, https://addons.mozilla.org/addon/8150)

Comment 1

[Scoobidiver (away)]

(In reply to Marcia Knous [:marcia] from comment #0)
> Seen while looking at the explosive report. This signatures seems to be new
> to 11.0B3.
Its address moves across versions so it exists in 11.0b1 and 11.0b2.

It's #44 top browser crasher in 11.0b3.

Comment 2

[Scoobidiver (away)]

I added the signature in 12.0 and 13.0.

Comment 3

[Scoobidiver (away)]

There's a spike in crashes (jumped from around 3 crashes/build to around 12 crashes/build) from 13.0a1/20120301, making it #15 top crasher in 13.0a1 over the last day.

The regression window for the spike is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=30b4f99a137c&tochange=1c3b291d0830

Here are correlations per extension in 13.0a1:
* Feb 3:
  je_free | JSObject::finish(JSContext*)|EXCEPTION_ACCESS_VIOLATION_READ (16 crashes)
     81% (13/16) vs.   5% (89/1954) [e-mail] (Yandex.Bar, https://addons.mozilla.org/addon/3495)
* Feb 4:
  je_free | JSObject::finish(JSContext*)|EXCEPTION_ACCESS_VIOLATION_READ (15 crashes)
     67% (10/15) vs.   8% (127/1517) [e-mail] (Yandex.Bar, https://addons.mozilla.org/addon/3495)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=je_free%20|%20JSObject%3A%3Afinish%28JSContext*%29

Comment 4

[Alex Keybl [:akeybl]]

Including Max to see if he has any insight into the high correlation with Yandex. Tracking for FF13. Should we also track for 12, or is this not a top crasher there?

Comment 5

[Scoobidiver (away)]

(In reply to Alex Keybl [:akeybl] from comment #4)
> Including Max to see if he has any insight into the high correlation with
> Yandex. Tracking for FF13. Should we also track for 12, or is this not a top
> crasher there?
It's not a top crasher in 11.0 or 12.0 (around #50).

Comment 6

[Scoobidiver (away)]

It's #9 top crasher in 13.0a2. The correlation to Yandex Bar is confirmed.

Comment 7

[Sheila Mooney]

Kev, can we get someone at Yandex to help? I know there are been a couple of people we cc'd on bugs before.

Comment 8

[Scoobidiver (away)]

From 14.0a1/20120405105200, the new crash signature is je_free | JSObject::finish(js::FreeOp*):
https://crash-stats.mozilla.com/report/list?signature=je_free+|+JSObject%3A%3Afinish%28js%3A%3AFreeOp*%29

Comment 9

[Scoobidiver (away)]

I add the 64-bit signature.

Comment 10

[Scoobidiver (away)]

I cleaned up old signatures.

Comment 11

[Alex Keybl [:akeybl]]

(In reply to Sheila Mooney from comment #7)
> Kev, can we get someone at Yandex to help? I know there are been a couple of
> people we cc'd on bugs before.

The Yandex devs most times look to us for guidance on where new problems lie. Other than code inspection of related functions on the stack, our only angle of attack is trying to get STR. Given the confirmed correlation in Comment 6, adding qawanted to this bug.

Comment 12

[David Mandelin [:dmandelin]]

JSObject::finish(JSContext *cx)
{
    if (hasDynamicSlots())
        cx->free_(slots);
    if (hasDynamicElements())
        cx->free_(getElementsHeader()); 
}

This is crashing on the second call to free. It could theoretically be a regression from bug 693221, but that landed quite a while ago so I'd be surprised if it would spike in 13 because of that.

The correlation with Yandex is partial in most recent data I found--only 35% of these crahes have Yandex installed.

Brian, does anything jump out at you as being likely to cause this crash? Is there any path that could store a bad pointer there, or double-free?

Alex: Does Yandex have binary components that use JSAPI, or anything like that? If they are interacting with the engine directly I imagine something could have changed that would break them. Otherwise it must have something to do with arrays, but I don't imagine that would help them very much.

Comment 13

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I was looking at the Yandex code for a different bug. The latest version doesn't appear to have any binary components. It does do a bunch of crazy XPCOM junk, though.

Comment 14

[u279076]

(In reply to David Mandelin from comment #12)
> JSObject::finish(JSContext *cx)
> {
>     if (hasDynamicSlots())
>         cx->free_(slots);
>     if (hasDynamicElements())
>         cx->free_(getElementsHeader()); 
> }
> 
> This is crashing on the second call to free. It could theoretically be a
> regression from bug 693221, but that landed quite a while ago so I'd be
> surprised if it would spike in 13 because of that.

Do we know of any particular actions I can take to try to hit that code using Yandex in Firefox?

Comment 15

[u279076]

I've been trying to test this for the last hour in Firefox 13.0b2 on Windows XP with Yandex 6.8 and have yet to experience a crash. I signed up for a Yandex account so I could test some of the toolbar functions and haven't been able to crash yet.

Comment 16

[David Mandelin [:dmandelin]]

Jeff, the crash is here:

inline void
JSObject::finish(js::FreeOp *fop)
{
    if (hasDynamicSlots())
        fop->free_(slots);
    if (hasDynamicElements())
        fop->free_(getElementsHeader());
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^         crash here
}

I also found this in vm/ObjectImpl.h:

    inline bool hasDynamicElements() const {
        /*
         * Note: for objects with zero fixed slots this could potentially give
         * a spurious 'true' result, if the end of this object is exactly
         * aligned with the end of its arena and dynamic slots are allocated
         * immediately afterwards. Such cases cannot occur for dense arrays
         * (which have at least two fixed slots) and can only result in a leak.
         */
        return elements != emptyObjectElements && elements != fixedElements();
    }

Wouldn't spurious 'true' cause a crash in JSObject::finish?

Comment 17

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I think the comment should say "a spurious 'false' result". So it shouldn't crash, it should just leak, as the comment says.

Comment 18

[u279076]

Attempts to reproduce this crash in QA have been unsuccessful to date. Is there another angle we can take?

Comment 19

[Alex Keybl [:akeybl]]

(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #18)
> Attempts to reproduce this crash in QA have been unsuccessful to date. Is
> there another angle we can take?

Nothing more from the QA angle if we haven't been able to reproduce.

Comment 20

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I was going through crashes today and I noticed that all the ones with this signature happen on the background finalize thread. I wonder if there's some kind of race here.

Comment 21

[Scoobidiver (away)]

It's #38 top browser crasher in 12.0, #33 in 13.0 Beta, #10 in 14.0a2, and #67 in 15.0a1.

One comment says:
"Aurora crashes after every attempt to exit by clicking the [x] button on the status bar. On the other hand, File->Exit ot Alt+F4 results in normal shutdowns with no crash alerts and reports." (bp-fa7bb4bd-0715-49bd-9e6e-418492120410)

The current correlations are:
*12.0: Nothing

*13.0 Beta:
  je_free | JSObject::finish(JSContext*)|EXCEPTION_ACCESS_VIOLATION_READ (115 crashes)
     34% (39/115) vs.   2% (835/38737) [e-mail] (Yandex.Bar, https://addons.mozilla.org/addon/3495)
     15% (17/115) vs.   2% (761/38737) {37964A3C-4EE8-47b1-8321-34DE2C39BA4D}
     14% (16/115) vs.   7% (2879/38737) {d10d0bf8-f5b5-c8b4-a8b2-2b9879e08c5d} (Adblock Plus, https://addons.mozilla.org/addon/1865)

*14.0a2:
  je_free | JSObject::finish(js::FreeOp*)|EXCEPTION_ACCESS_VIOLATION_READ (37 crashes)
     73% (27/37) vs.   3% (109/3154) [e-mail] (Yandex.Bar, https://addons.mozilla.org/addon/3495)
     16% (6/37) vs.   4% (114/3154) {37964A3C-4EE8-47b1-8321-34DE2C39BA4D}
     14% (5/37) vs.   3% (84/3154) {1018e4d6-728f-4b20-ad56-37578a4de76b} (Flagfox, https://addons.mozilla.org/addon/5791)

Comment 22

[David Mandelin [:dmandelin]]

(In reply to Bill McCloskey (:billm) from comment #20)
> I was going through crashes today and I noticed that all the ones with this
> signature happen on the background finalize thread. I wonder if there's some
> kind of race here.

Would it make sense to turn off the background finalize thread to see if that helps? What would be the right channel for that?

Comment 23

[David Mandelin [:dmandelin]]

(In reply to David Mandelin from comment #22)
> (In reply to Bill McCloskey (:billm) from comment #20)
> > I was going through crashes today and I noticed that all the ones with this
> > signature happen on the background finalize thread. I wonder if there's some
> > kind of race here.
> 
> Would it make sense to turn off the background finalize thread to see if
> that helps? What would be the right channel for that?

Or, could we event turn off the background finalize thread if Yandex toolbar is present?

Comment 24

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to David Mandelin from comment #22)
> Would it make sense to turn off the background finalize thread to see if
> that helps? What would be the right channel for that?

I don't think that turning it off would help us track down the source of the bug. Even without background finalization, the space of what can go wrong seems really large.

However, if we're interested in just making the bug go away, then this might be worth trying. Turning it off just with Yandex would probably require some pretty gross hacks, though.

However, are we even confident that something regressed here? The crash ranking didn't really change much between 12 and 13. Is it a regression because of there's a Yandex correlation in 13 and not in 12?

Comment 25

[Scoobidiver (away)]

(In reply to Bill McCloskey (:billm) from comment #24)
> I don't think that turning it off would help us track down the source of the
> bug. Even without background finalization, the space of what can go wrong
> seems really large.
The correlation varies a lot, from 34% to 73% depending on the channel and the day, so I don't think it will help to hack the code.

> However, are we even confident that something regressed here? The crash
> ranking didn't really change much between 12 and 13. Is it a regression
> because of there's a Yandex correlation in 13 and not in 12?
It seems indeed that the je_free | JSObject::finish signature that appeared in 13.0 is now at the same volume as the je_free | mozglue.dll signature that appeared in 11.0. The high crash volume in Aurora is still unexplained.

Comment 26

[Scoobidiver (away)]

15.0a2 and 16.0a1 seem unaffected.

Comment 27

[Scoobidiver (away)]

There are no crashes in 15.0.1 and above.