Bugzilla@Mozilla – Bug 513981
TM: possible GC rooting bug with Google Docs
Last modified: 2010-01-25 00:06:06 PST
Summon comment box
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 <gal@mozilla.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.
*** Bug 514766 has been marked as a duplicate of this bug. ***
Created attachment 398787 [details] [review] Patch 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.
Great initial debugging by dmandelin proving the source of the bug.
Created attachment 398793 [details] [review] patch Make sure native calls immediately unroot nativeVp as well, not just setter/getter invocations.
http://hg.mozilla.org/tracemonkey/rev/18ec7c6f7fb2
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.
Can we get this landed on trunk? Google Docs is currently almost impossible to use. :/
http://hg.mozilla.org/mozilla-central/rev/18ec7c6f7fb2
(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.
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.
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cd81bbde4522
Comment on attachment 398793 [details] [review] patch Approved for 1.9.1.5, 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.
Created attachment 410346 [details] [review] patch
1.9.1 patch attached. Passes trace tests.
Comment on attachment 410346 [details] [review] patch Moving 1.9.1.6 approval to the correct branch patch
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0c83a08adbf6
Verified for 1.9.1.6 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.6pre) Gecko/20091119 Shiretoko/3.5.6pre (.NET CLR 3.5.30729) using Google Docs (with JIT enabled).