This is a secondary bug from bug 504797. To reproduce, use this rev with a debug build: changeset: 32107:e8a55ae8fb98 tag: qparent user: Andreas Gal <email@example.com> date: Mon Aug 31 17:02:16 2009 -0700 summary: Compilation fix for bug 513787. and create a new document in Google Docs. You may get a slow script dialog, but if you continue it will crash eventually inside array_slice called from trace. The problem is that the array created by js_NewArrayObject at the end of array_slice (the non-dense case) gets GC'd (and its fields overwritten) while it is being populated in the loop. I showed that by printing field values immediately after allocation and then inside SetArrayElement. I also tried adding a new JSAutoTempValueRooter right after the call to js_NewArrayObject and that stopped the problem.
We used to do the builtinStatus guard before clearing nativeVp and nativeVplen on trace. That has now switched around, so this patch is safe both ways.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #398787 - Flags: review?(gal)
Great initial debugging by dmandelin proving the source of the bug.
Make sure native calls immediately unroot nativeVp as well, not just setter/getter invocations.
Attachment #398787 - Attachment is obsolete: true
Attachment #398793 - Flags: review+
Whiteboard: fixed-in-tracemonkey → fixed-in-tracemonkey [sg:critical?]
Attachment #398793 - Flags: approval22.214.171.124? → approval126.96.36.199?
We'll switch this to blocking the next specific 1.9.1.x after it lands on trunk and 1.9.2 successfully.
The dependent bug has to be landed at the same time.
Whiteboard: fixed-in-tracemonkey [sg:critical?] → fixed-in-tracemonkey [sg:critical]
Can we get this landed on trunk? Google Docs is currently almost impossible to use. :/
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
(In reply to comment #0) > This is a secondary bug from bug 504797. To reproduce, use this rev with a > debug build: What does this mean? A regression from 504797? something found while you were investigating that? Bug 504797 appears to be a regression itself, but pinned on a checkin that would seem to say the 1.9.1 branch is unaffected. Is this bug and bug 514999 independent of that regression, and needed on the 1.9.1 branch anyway?
(In reply to comment #10) > (In reply to comment #0) > > This is a secondary bug from bug 504797. To reproduce, use this rev with a > > debug build: > > What does this mean? A regression from 504797? something found while you were > investigating that? Bug 504797 appears to be a regression itself, but pinned on > a checkin that would seem to say the 1.9.1 branch is unaffected. Exactly what I meant is: - I found this bug while debugging bug 504797. - This bug was exposed by bug 504797. > Is this bug and bug 514999 independent of that regression, and needed on the > 1.9.1 branch anyway? I don't have access to bug 514999. This bug is independent of bug 504797, by which I mean that fixing bug 504797 does not necessarily fix this bug. But I don't have a way of reproducing this bug without bug 504797 active.
Attachment #398793 - Flags: approval188.8.131.52? → approval184.108.40.206?
marcia crashed with a related stack in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=517957 We should get this onto 1.9.2 so QA doesn't spend cycles chasing stuff we fixed.
Where are we with getting this on 1.9.2, especially since it's a beta blocker.
Comment on attachment 398793 [details] [diff] [review] patch Approved for 220.127.116.11, a=dveditz for release-drivers
Andreas, this doesn't apply to 1.9.1. Can you backport?
Ok. Same warning here. 191 is missing the getters/setters patch. This patch fixes a gc hazard in the getters/setters patch. I have to remove that code from this patch to make it fit 191. If we ever land getters/setters on 191, bad things will happen if we forget to backport the missing piece. Otherwise this applies somewhat sanely. Patch in a sec.
1.9.1 patch attached. Passes trace tests.
Attachment #410346 - Flags: review?(mrbkap) → review+
Comment on attachment 410346 [details] [diff] [review] patch Moving 18.104.22.168 approval to the correct branch patch
Verified for 22.214.171.124 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:126.96.36.199pre) Gecko/20091119 Shiretoko/3.5.6pre (.NET CLR 3.5.30729) using Google Docs (with JIT enabled).
Bug in removed tracer code, setting in-testsuite- flag.
You need to log in before you can comment on or make changes to this bug.