Last Comment Bug 340129 - crashes [@ 0x20202020] within 10-20 deep recursion of MarkSharpObjects
: crashes [@ 0x20202020] within 10-20 deep recursion of MarkSharpObjects
Status: RESOLVED FIXED
[sg:critical?][need testcase]
: crash, fixed1.8.0.5, fixed1.8.1, topcrash
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: 1.8 Branch
: All All
: -- critical (vote)
: ---
Assigned To: Igor Bukanov
:
:
Mentors:
: 340655 (view as bug list)
Depends on:
Blocks: 341315
  Show dependency treegraph
 
Reported: 2006-06-02 10:50 PDT by David Baron :dbaron: ⌚️UTC-7
Modified: 2011-06-13 10:01 PDT (History)
12 users (show)
dveditz: blocking1.8.0.5+
bob: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Extra rooting in MarkSharpObjects (3.25 KB, patch)
2006-06-11 03:57 PDT, Igor Bukanov
no flags Details | Diff | Splinter Review
Extra rooting in MarkSharpObjects v2 (3.34 KB, patch)
2006-06-11 04:09 PDT, Igor Bukanov
mrbkap: review+
mrbkap: approval‑branch‑1.8.1+
Details | Diff | Splinter Review
Extra rooting in MarkSharpObjects v3 (3.49 KB, patch)
2006-06-13 06:27 PDT, Igor Bukanov
mrbkap: review+
mrbkap: approval‑branch‑1.8.1+
Details | Diff | Splinter Review
Patch to commit (3.49 KB, patch)
2006-06-13 15:26 PDT, Igor Bukanov
no flags Details | Diff | Splinter Review
Patch for 1.8.1, 1.8.0 branches (3.86 KB, patch)
2006-06-13 15:43 PDT, Igor Bukanov
dveditz: approval1.8.0.5+
Details | Diff | Splinter Review
1.0.x patch (3.16 KB, patch)
2006-08-08 08:15 PDT, Alexander Sack
no flags Details | Diff | Splinter Review

Description David Baron :dbaron: ⌚️UTC-7 2006-06-02 10:50:37 PDT
I looked at the 0x20202020 crashes in Firefox 1.5.0.4.  Out of the last 500 talkback reports, there were 7 such crashes (1 Linux, 6 Windows; many of the windows ones show up as xpsp2res.dll due to the 0x20000000 base address).

The *bottom* of the stack was always identical (except for the platform variation in how nsTimerImpl::Fire is called):

MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
js_EnterSharpObject  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 539]
js_obj_toSource  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 672]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1188]
js_InternalInvoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1285]
js_TryMethod  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 4073]
js_ValueToSource  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsstr.c, line 2804]
str_uneval  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsstr.c, line 477]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3584]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
fun_apply  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 1669]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1188]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3584]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3584]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
fun_apply  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 1669]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1188]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3584]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
nsXPCWrappedJSClass::CallMethod  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp, line 1373]
nsXPCWrappedJS::CallMethod  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappedjs.cpp, line 462]
SharedStub  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp, line 147]
nsTimerImpl::Fire  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/xpcom/threads/nsTimerImpl.cpp, line 407]
nsAppStartup::Run  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 151]
main  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/browser/app/nsBrowserApp.cpp, line 61]

but the top of the stack varies a bit, as does the number of levels of MarkSharpObjects recursion.  Examples are:

xpsp2res.dll + 0x202020 (0x20202020)
JS_GetReservedSlot  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3328]
RewrapIfDeepWrapper  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp, line 294]
XPC_NW_GetOrSetProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp, line 588]
XPC_NW_GetProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp, line 595]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 472]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]


and


xpsp2res.dll + 0x202020 (0x20202020)
JS_GetReservedSlot  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3328]
XPC_NW_AddProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp, line 248]
js_DefineNativeProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 2567]
js_DefineProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 2452]
DefineUCProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 2383]
JS_DefineUCProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 2787]
nsWindowSH::NewResolve  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/dom/src/base/nsDOMClassInfo.cpp, line 5793]
XPC_NW_NewResolve  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp, line 756]
js_LookupPropertyWithFlags  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 2738]
js_LookupProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 2643]
XPC_NW_Enumerate  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/XPCNativeWrapper.cpp, line 631]
js_Enumerate  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 3495]
JS_Enumerate  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3076]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 442]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]

The deep recursion in MarkSharpObjects seems suspicious to me, although maybe there's a good reason for it.

This seems like something GC_MARK_DEBUG might catch; perhaps there's nothing in the test suite testing toSource on objects that have getters and setters within them.
Comment 1 David Baron :dbaron: ⌚️UTC-7 2006-06-02 11:12:47 PDT
FWIW, 2 of them didn't involve any XPC_NW stuff at the top:

0x20202020
fun_resolve()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsfun.c, line 1126]
js_LookupPropertyWithFlags()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsobj.c, line 2739]
js_LookupProperty()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsobj.c, line 2644]
fun_enumerate()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsfun.c, line 1044]
js_Enumerate()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsobj.c, line 3495]
JS_Enumerate()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsapi.c, line 3072]
MarkSharpObjects()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsobj.c, line 442]
MarkSharpObjects()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsobj.c, line 481]
MarkSharpObjects()  [/builds/tinderbox/Fx-Mozilla1.8.0-Release/Linux_2.4.21-37.EL_Depend/mozilla/js/src/jsobj.c, line 481]


xpsp2res.dll + 0x202020 (0x20202020)
js_LookupProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 2643]
fun_enumerate  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 1044]
js_Enumerate  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 3495]
JS_Enumerate  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3076]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 442]
MarkSharpObjects  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 482]
Comment 2 David Baron :dbaron: ⌚️UTC-7 2006-06-02 11:15:23 PDT
Sorry, I got the line numbers wrong; the recursion wasn't in the getter case, so it's less suspicious.
Comment 3 Peter Lowe 2006-06-06 12:28:01 PDT
I have experienced this crash quite regularly since the update to Firefox 1.5.0.4.  I experience it on my dual Xeon (katmai 550mHz) machine, but not my AMD Sempron T-bred, both of which use identical Firefox setups.  I have tried various things to isolate the bug down...i discovered it doesn't happen on the dual CPU box with all extensions disabled, but, as i re-enabled them one by one, when I re-enabled NoScrpit, it started happening again.  When I disabled all extensions BUT NoScript, it didn't crash, but spawned a new window without user intervention, three times in four hours (could be unrelated).  I can also replicate this with a clean profile, on the dual CPU box.  TalkBacks for the crashes I had are TB19569407E most recent, TB19543546W TB19522696M TB19522455Q TB19502213H.  Most recent was with CPU affinity for Firefox set to one CPU only, and with a clean profile.  Pages I used to test this were:  http://www.google.ca/ig, http://www.slashdot.org, http://www.gmail.com, a random wikipedia page, and http://news.google.ca.  I feel (with no proof) that it is somehow related to AJAXy pages like gmail, refreshing, perhaps in combination with one or more Extensions.

The crash does not occur at all, under the same conditions, on my AMD Sempron T-Bred machine, ever.  Only happens on the dual CPU machine, running Windows XP SP2 (as does the AMD).  Hope this helps.
Comment 4 Daniel Veditz [:dveditz] 2006-06-06 13:45:32 PDT
> as i re-enabled them one by one, when I re-enabled NoScrpit, it started
> happening again.  When I disabled all extensions BUT NoScript, it didn't

Could list the extensions you had enabled at the NoScript+others point? The bug is almost certainly in Firefox (especially if nothing else changed but the upgrade to 1.5.0.4) but if we can find a group of extensions that trigger it more often it will help us track it down.

> Most recent was with CPU affinity for Firefox set to one CPU
> only, and with a clean profile.

What do you mean by "clean profile" given you earlier said you didn't see it without extensions? Did you create a *new* profile and add the extensions, or just try to "clean" your existing profile? If the latter, which bits did you clean?

Thanks for the info, this may give us a starting point.
Comment 5 Peter Lowe 2006-06-06 16:10:32 PDT
(In reply to comment #4)
> Could list the extensions you had enabled at the NoScript+others point? The bug
> is almost certainly in Firefox (especially if nothing else changed but the
> upgrade to 1.5.0.4) but if we can find a group of extensions that trigger it
> more often it will help us track it down.

Unluckily, NoScript was the second-last extension I had re-enabled, but here's the list:
DOM Inspector 1.8.0.4
Talkback 1.5.0.4
Cache Status 0.6.3
Adblock Filterset.G Updater 0.3.0.4
UI Tweaker (Formerly Toolbar Cleanup) 1.7.0
Crash Recovery 0.6.8
Uppity 1.4.1
Image Zoom 0.2.5
Google Toolbar for Firefox 2.0.20060515W
Adblock Plus 0.5.11.3
PDF Download 0.7
Back to Top 3.0
Forecastfox Enhanced 0.9.0.2.1
Text Size Toolbar 0.4.1
ChatZilla 0.9.73
Duplicate Tab 0.7.3
NoScript 1.1.4.1
IE Tab 1.0.9
Tab History 1.0.3
Mozilla Calendar 0.2.0.20060116
Context Search 0.2.1
CacheIt! 0.9.7
Search Plugin Hacks 0.1.3
Tabbrowser Preferences 1.2.8.9
Right-Click-Link 1.1.1
FLST (Focus Last Selected Tab) 0.8.5
Menu Editor 1.2
WellRounded 0.43
Download manager Tweak 0.7.1
CustomizeGoogle 0.48
Disable Targets For Downloads 1.0.1

I recently noticed the version 2 Google Toolbar can detect if you are using Gmail in Firefox, and pops up a Clippy-esqe dialog asking you if you want to set default mailto: links to Gmail...don't know if that's related, but every crash of this type has occurred with Gmail either loaded or focused.

> What do you mean by "clean profile" given you earlier said you didn't see it
> without extensions? Did you create a *new* profile and add the extensions, or
> just try to "clean" your existing profile? If the latter, which bits did you
> clean?

I closed Firefox, went into my profile, moved prefs.js to my desktop and renamed it, and then re-opened Firefox to generate a clean one.  Sorry, I should have been more precise.  This had the effect of slowing the frequency of the crash (from within half an hour of browser opening to several hours of stability before crash).

LMK if you want me to try any other steps...I'd be glad to be a guinea pig.
Comment 6 David Baron :dbaron: ⌚️UTC-7 2006-06-08 14:16:37 PDT
Any chance you could try disabling the extensions one by one?  You could then see which disabling makes it stop, then reenable that one that made the crash stop and keep going until you get to the minimum set of extensions enabled that causes the crash.
Comment 7 Jay Patel [:jay] 2006-06-08 14:55:46 PDT
Not going to make 1.8.0.5 since we still have not been able to narrow down the testcase.  Please continue to debug and once we have more information, perhaps we can find a fix for the next release.
Comment 8 Igor Bukanov 2006-06-09 12:48:48 PDT
My current theory is that 0x20202020 comes from 4 GC flags misread as slot pointer. When GC thing is finalized, the word corresponding to JSObject.slots is replaced by a pointer to thing's flags. If subsequently the code will try to access JSObject.slots[someSlot], it would effectively reinterpret the flag byte as a pointer and crash. Now 0x20 is precisely GCF_FINAL which means that 0x20202020 indicates a chunk of 4 free GC cells.

The theory is even more sound if one take into account that the stack traces come from the code that tries to access  JSObject.slots. 

The only problem is why it does not crash earlier when JSObject.map is accessed. During GC that field is replaced by a pointer to the next free GC cell and the code must crash earlier.
Comment 9 Igor Bukanov 2006-06-11 03:57:36 PDT
Created attachment 225175 [details] [diff] [review]
Extra rooting in MarkSharpObjects

I am not sure if this patch would address the problems leading to the bug but it fixes the improper assumption in MarkSharpObjects that its obj argument is rooted and stays rooted.
Comment 10 Igor Bukanov 2006-06-11 04:09:01 PDT
Created attachment 225176 [details] [diff] [review]
Extra rooting in MarkSharpObjects v2

The previous patch was too quick and would stop enumeration after the first property.
Comment 11 Blake Kaplan (:mrbkap) 2006-06-12 15:11:44 PDT
Comment on attachment 225176 [details] [diff] [review]
Extra rooting in MarkSharpObjects v2

>+                             * But since during the recursive invocation of
>+                             * MarkSharpObjects the getter can be replaced

brendan-style-nit: Perhaps switch this sentence to be in the active voice ('Since the call to MarkSharpObjects can replace the getter ....')

>+                             * we have to root the value to realize
>+                             * MarkSharpObjects assumption that its argument
>+                             * is rooted.

nit: I'm not a big fan of 'realize' in this situation (here and below), I prefer to 'enforce' my assumptions.

r=mrbkap
Comment 12 Daniel Veditz [:dveditz] 2006-06-12 19:26:39 PDT
bug 341315 is a 1.5.0.4 top-crash in nsHTMLDocument::MatchLinks, also showing a long string of MarkSharpObjects calls in the middle of the stack. Maybe this fix will help there too?
Comment 13 Igor Bukanov 2006-06-13 06:27:32 PDT
Created attachment 225419 [details] [diff] [review]
Extra rooting in MarkSharpObjects v3

Closed tree gives time to find more problems ;) 

The previous patch was not enough to completely address issues rising from running GC during Object.prototype.toSource(). Although the extra problem (see comments in the patch for details) could just lead to toSource() producing a garbage AFAICS , it is better to fix this once.
Comment 14 Blake Kaplan (:mrbkap) 2006-06-13 10:28:50 PDT
Comment on attachment 225419 [details] [diff] [review]
Extra rooting in MarkSharpObjects v3

Please replace both instances of "JSTempValueRoote" with "JSTempValueRooter" (plural, when appropriate, of course).
Comment 15 David Baron :dbaron: ⌚️UTC-7 2006-06-13 10:49:23 PDT
Do you have testcases that show crashes with GC_MARK_DEBUG?  If so, could they be added to the test suite?
Comment 16 Tracy Walker [:tracy] 2006-06-13 12:28:58 PDT
might bug 340655 (crashes while browsing [@ JS_GetReservedSlot()] with google toolbar) be  adupe of this?
Comment 17 Daniel Veditz [:dveditz] 2006-06-13 15:02:19 PDT
Please land on trunk and 1.8 branch so we can verify this reduces the talkback incidents.
Comment 18 Igor Bukanov 2006-06-13 15:26:37 PDT
Created attachment 225487 [details] [diff] [review]
Patch to commit

This is a version of the patch from comment 13 with spelling fixes.
Comment 19 Igor Bukanov 2006-06-13 15:31:02 PDT
I committed to the trunk the patch from comment 18. Hopefully this indeed fixes the stack traces.
Comment 20 Igor Bukanov 2006-06-13 15:43:25 PDT
Created attachment 225492 [details] [diff] [review]
Patch for 1.8.1, 1.8.0 branches

This version of the patch adjusts to the fact that on branches GC_MARK has 4, not 4, arguments.
Comment 21 Igor Bukanov 2006-06-13 15:46:26 PDT
I committed the patch from comment 20 to MOZILLA_1_8_BRANCH
Comment 22 David Baron :dbaron: ⌚️UTC-7 2006-06-13 16:09:16 PDT
This was backed out of the trunk since the tree is closed.

Worth requesting 1.8.0.5 approval as well.
Comment 23 Igor Bukanov 2006-06-13 16:26:01 PDT
Comment on attachment 225492 [details] [diff] [review]
Patch for 1.8.1, 1.8.0 branches

The patch applies to MOZILLA_1_8_0 branch as is.
Comment 24 Igor Bukanov 2006-06-13 16:35:24 PDT
(In reply to comment #15)
> Do you have testcases that show crashes with GC_MARK_DEBUG?

No. The test case can not be pure shell since none of the standard objects has getProperty hook that can return newly allocated objects. I will try to create one for the browser.



Comment 25 Daniel Veditz [:dveditz] 2006-06-14 15:08:32 PDT
Comment on attachment 225492 [details] [diff] [review]
Patch for 1.8.1, 1.8.0 branches

approved for 1.8.0 branch, a=dveditz for drivers
Comment 26 Igor Bukanov 2006-06-14 15:46:32 PDT
I committed the patch from comment 20 to MOZILLA_1_8_0_BRANCH.
Comment 27 Igor Bukanov 2006-06-15 03:19:03 PDT
I committed the patch from comment 18 to the trunk.
Comment 28 timeless 2006-06-18 05:40:33 PDT
*** Bug 340655 has been marked as a duplicate of this bug. ***
Comment 29 Alexander Sack 2006-08-08 08:15:13 PDT
Created attachment 232725 [details] [diff] [review]
1.0.x patch

Note You need to log in before you can comment on or make changes to this bug.