893513 - Crash with undoManager recursion

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Attachment: testcase

With:
  user_pref("dom.undo_manager.enabled", true);

the testcase recurses deeply and then hits a MOZ_CRASH in JS_GetFunctionScript.  The MOZ_CRASH was added by Till in bug 815010.

This might be related to bug 813646.

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Scoobidiver (away)]

On Windows: bp-cce2a880-b6db-4c54-a525-93c6c2130714.

Comment 3

[Robert Kaiser]

Given the signature, can you try this with a build after bug 885668 landed?

Comment 4

[Jesse Ruderman]

JavaScript error: , line 0: too much recursion
JavaScript error: file:///Users/jruderman/Desktop/r-1.html, line 15: too much recursion
JavaScript error: file:///Users/jruderman/Desktop/r-1.html, line 15: too much recursion

Do you think bug 885668 fixed the underlying bug here, or just altered the value of
(stack space % stack used by each recursion)?

Comment 5

[Bobby Holley (:bholley)]

(In reply to Jesse Ruderman from comment #4)
> Do you think bug 885668 fixed the underlying bug here, or just altered the
> value of
> (stack space % stack used by each recursion)?

If that crash turned into an over-recursion error after my fix, that probably indicates that the original problem was that we were bumping up against the native stack limit when trying to do certain JSAPI operations that we were assuming to be infallible.

bug 813646 is probably what we need to fix these crashes in the general case.

Comment 6

[Scoobidiver (away)]

(In reply to Bobby Holley (:bholley) from comment #5)
> bug 813646 is probably what we need to fix these crashes in the general case.
So can we close this bug as fixed by bug 885668's patch or should we rename it into "too much recursion" error?

Comment 7

[Bobby Holley (:bholley)]

(In reply to Scoobidiver from comment #6)
> (In reply to Bobby Holley (:bholley) from comment #5)
> > bug 813646 is probably what we need to fix these crashes in the general case.
> So can we close this bug as fixed by bug 885668's patch or should we rename
> it into "too much recursion" error?

I think can close it.

Comment 8

[Ada [:adalucinet]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:26.0) Gecko/20100101 Firefox/26.0

Verified as fixed with Firefox 24 beta 8 and latest Nightly (2013-09-02): 0 crashes when using the attached testcase.
Report from Socorro regarding JS_GetFunctionScript(JSContext*, JSFunction*) signature: 1 crash on beta 6 and 1 crash on beta 7.

Comment 9

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 25 beta 1 - Firefox does not crash when using the test case attached in the Description. Also, checked the Socorro reports and there are no crashes on Firefox 25.

Verified on Windows 7, Ubuntu 13.04 and Mac OS X 10.8:
Mozilla/5.0 (Windows NT 6.1; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (X11; Linux i686; rv:25.0) Gecko/20100101 Firefox/25.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0
Build ID: 20130917123208