734129 - Assertion failure: enumerators == cx->enumerators

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

1. http://www.boston.com/bigpicture/2012/03/smithsonian_magazine_photo_con.html
2. Dismiss slow script dialog.
3. Assertion failure: enumerators == cx->enumerators, at /work/mozilla/builds/nightly/mozilla/js/src/jsinterp.cpp:453

Seen on Nightly/13 Mac and Windows so far.

Reproducible with a saved version. I'll try to reduce.

Comment 1

[Bob Clary [:bc] (inactive)]

Disabling the chrome and dom script timeout hangs the browser on Mac but does not reproduce the assertion. Note when I say dismiss slow script dialog, I mean to allow the script to continue. I'll check on Windows when I have builds available.

Note crashes today's Nightly/13: bp-2840263e-b4d4-4f68-ba38-25d672120308
Firefox 13.0a1 Crash Report [@ js_SuppressDeletedProperty ]

Comment 2

[Scoobidiver (away)]

It's currently #8 top crasher in 13.0a1 over the last day.

Comment 3

[Bob Clary [:bc] (inactive)]

Automation is continuing to pick up more sites with this assertion. I'll see about getting one that doesn't involve the slow script dialog that I can reduce.

Comment 4

[Luke Wagner [:luke]]

Ah, it seems that uncatchable exceptions completely skip the exception-unwind logic and thus don't call CloseIterator and thus leave a dangling cx->enumerators.  That explains how this is a top crash: any slow-script under for-in will crash.

Comment 5

[David Mandelin [:dmandelin]]

Does this affect 12 or older?

Comment 6

[Bob Clary [:bc] (inactive)]

Just Nightly/13, not Aurora/12 or Beta/11.

Comment 7

[Luke Wagner [:luke]]

Attachment: fix and test

This patch fixes the bug by having the exception handling pop cx->enumerators even for uncatchable exceptions.  I added a jsapi-test that hits the bug both in the interpreter and jit.

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 604560 [details] [diff] [review]
fix and test

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

<3 TryNoteIter

Comment 9

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/412f24be91ed

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/412f24be91ed

Comment 11

[Scoobidiver (away)]

I see one crash after the patch landed: bp-66d44218-ba53-49c2-985c-66fb62120312.

Comment 12

[Bob Clary [:bc] (inactive)]

I retested the 11 urls where I have seen this and did not get this assertion. I did get:

Assertion failure: compartment()->activeInference (see bug 727476 and bug 734972)
on

http://transportationnation.org/2012/03/08/here-we-go-senate-set-to-start-highway-bill-votes-thursday/
with Beta/11 Windows XP|Windows 7, Nightly/13 Windows XP

http://www.boston.com/bigpicture/2012/03/smithsonian_magazine_photo_con.html
with Beta/11 Windows XP|Windows 7, Aurora/12 Windows XP

and one Crash |  ASSERTION: You can't dereference a NULL nsAutoPtr with operator->().: 'mRawPtr != 0'
on http://shar.es/gDLDB (http://transportationnation.org/2012/03/06/proposed-ny-mta-hikes-wont-pay-for-service-upgrades/) with Beta/11 on Windows 7

Comment 13

[Lukas Blakk [:lsblakk] use ?needinfo]

Please nominate for uplift to Aurora (13) as per comment 6 stating it is affected.

Comment 14

[Luke Wagner [:luke]]

412f24be91ed is already on Aurora (13).

Comment 15

[Paul Silaghi, QA [:pauly]]

No crashes on FF 13b4 and FF 2012-05-18-mozilla-beta-debug on Win 7, Ubuntu 12.04 and Mac OS X 10.6. Marking verified.