Closed Bug 893513 Opened 7 years ago Closed 7 years ago

Crash with undoManager recursion

Categories

(Core :: JavaScript Engine, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla25
Tracking Status
firefox24 --- verified
firefox25 --- verified

People

(Reporter: jruderman, Unassigned)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(2 files)

Attached file 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.
Attached file stack
On Windows: bp-cce2a880-b6db-4c54-a525-93c6c2130714.
Crash Signature: [@ JS_GetFunctionScript(JSContext*, JSFunction*) ]
Depends on: 885668
OS: Mac OS X → All
Hardware: x86_64 → All
Given the signature, can you try this with a build after bug 885668 landed?
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)?
Flags: needinfo?(bobbyholley+bmo)
(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.
Flags: needinfo?(bobbyholley+bmo)
(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?
(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.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
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.
Keywords: verifyme
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
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.