Closed
Bug 306788
Opened 19 years ago
Closed 19 years ago
Sorting array of arrays can crash due to GC
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla1.8beta4
People
(Reporter: bzbarsky, Assigned: brendan)
Details
(Keywords: js1.5, verified1.7.13, verified1.8)
Attachments
(1 file)
5.06 KB,
patch
|
mrbkap
:
review+
shaver
:
superreview+
|
Details | Diff | Splinter Review |
BUILD: Current trunk debug build STEPS TO REPRODUCE: 1) Load http://www.cloud9sales.com/test/raining_NSMozFail.htm 2) Click the "All Products" link NOTE: What the page is doing is: prodRainTapThr[0] = new Array("Airedale","","ATPAIR.jpg",125,151,"Multi","RcP","$59.95",339); ... prodRainTapThr[58] = new Array("Yorkie","","ATPYOR.jpg",150,114,"Multi","RcL","$59.95",396); prodRainTapThr.sort(); We crash inside Array.sort like so: (gdb) frame 0 #0 0xb7e6fb09 in js_CompareStrings (str1=0xb32460e0, str2=0x83ae120) at ../../../mozilla/js/src/jsstr.c:2800 2800 s1 = JSSTRING_CHARS(str1), s2 = JSSTRING_CHARS(str2); (gdb) p *str1 $9 = {length = 3671775962, chars = 0xdadadada} The problem is the code at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsarray.c&rev=3.60&mark=816,817#812 -- astr can get GCed when we create bstr. I've verified that if I lock astr around the creation of bstr the crash goes away.
Reporter | ||
Comment 1•19 years ago
|
||
At brendan's prodding, I broke in js_GC -- it's being called of a DOM branch callback with this stack: #4 0xb6884975 in nsJSContext::DOMBranchCallback (cx=0x88010a0, script=0x0) at ../../../../mozilla/dom/src/base/nsJSEnvironment.cpp:516 #5 0xb7e35636 in js_EnterSharpObject (cx=0x88010a0, obj=0xb29bcbd0, idap=0x0, sp=0xbfffd9d0) at ../../../mozilla/js/src/jsobj.c:507 #6 0xb7de2c69 in array_join_sub (cx=0x88010a0, obj=0xb29bcbd0, sep=0xb7e9ce44, literalize=0, rval=0xbfffdb40, localeString=0) at ../../../mozilla/js/src/jsarray.c:369 #7 0xb7de3319 in array_toString (cx=0x88010a0, obj=0xb29bcbd0, argc=0, argv=0xb2949d2c, rval=0xbfffdb40) at ../../../mozilla/js/src/jsarray.c:545 #8 0xb7e14d4f in js_Invoke (cx=0x88010a0, argc=0, flags=2) at ../../../mozilla/js/src/jsinterp.c:1174 #9 0xb7e15182 in js_InternalInvoke (cx=0x88010a0, obj=0xb29bcbd0, fval=-1259988056, flags=0, argc=0, argv=0x0, rval=0xbfffdd10) at ../../../mozilla/js/src/jsinterp.c:1271 #10 0xb7e3f5e4 in js_TryMethod (cx=0x88010a0, obj=0xb29bcbd0, atom=0x81f6620, argc=0, argv=0x0, rval=0xbfffdd10) at ../../../mozilla/js/src/jsobj.c:3890 #11 0xb7e3d58e in js_DefaultValue (cx=0x88010a0, obj=0xb29bcbd0, hint=JSTYPE_STRING, vp=0xbfffdd54) at ../../../mozilla/js/src/jsobj.c:3217 #12 0xb7e6f78a in js_ValueToString (cx=0x88010a0, v=-1298412592) at ../../../mozilla/js/src/jsstr.c:2739 #13 0xb7de3bda in sort_compare (a=0xb29c2b98, b=0x
Assignee | ||
Comment 2•19 years ago
|
||
Fixes the bug bz identified, plus an old bug where failure from js_ValueToNumber was not propagated. /be
Attachment #194628 -
Flags: superreview?(shaver)
Attachment #194628 -
Flags: review?(mrbkap)
Assignee | ||
Comment 3•19 years ago
|
||
Easy crash bug to fix. /be
Assignee: general → brendan
Flags: blocking1.8b4+
Keywords: js1.5
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta4
Comment 4•19 years ago
|
||
Comment on attachment 194628 [details] [diff] [review] fix r=mrbkap
Attachment #194628 -
Flags: review?(mrbkap) → review+
Comment on attachment 194628 [details] [diff] [review] fix sr=shaver
Attachment #194628 -
Flags: superreview?(shaver) → superreview+
Assignee | ||
Comment 6•19 years ago
|
||
Fixed on trunk and 1.8 branch. /be
Comment 7•19 years ago
|
||
Verified FIXED using trunk build 2005-09-04-06 on Windows XP SeaMonkey trunk by loading http://www.cloud9sales.com/test/raining_NSMozFail.htm and clicking "All Products" a few times. No crash.
Status: RESOLVED → VERIFIED
Comment 8•19 years ago
|
||
Checking in regress-306788.js; /cvsroot/mozilla/js/tests/js1_5/Regress/regress-306788.js,v <-- regress-306788.js initial revision: 1.1 done
Flags: testcase+
Comment 9•19 years ago
|
||
Fix checked into the 1.7 branch as part of my bug 311497 checkin.
Keywords: fixed-aviary1.0.8,
fixed1.7.13
Comment 10•18 years ago
|
||
hmmm, not crashing. But "all products" page never finishes loading *endless throbber spinning*. Is that expected?
Comment 11•18 years ago
|
||
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.13) Gecko/20060215 Firefox/1.0.8, seeing the same as Tracy, but no crash. Looks like that is a test page, so not sure how to answer Tracy's question, but if anyone knows, please share.
Keywords: fixed-aviary1.0.8 → verified-aviary1.0.8
Comment 12•18 years ago
|
||
js1_5/Regress/regress-306788.js verified on winxp/linux/mac, firefox 1.7.13, 1.8.0.1, 1.8, 1.9a1 verified on winxp, mozilla 1.7.13 note that I don't show a crash in the tests for firefox 1.0 or 1.0.7 on any platform for this test and don't crash in either 1.0 or 1.0.7 for the testcase when clicking the all products link.
You need to log in
before you can comment on or make changes to this bug.
Description
•