307317 - JS engine assert when running with WAY_TOO_MUCH_GC

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

Starting with WAY_TOO_MUCH_GC asserts in JS regexp code:

STEPS TO REPRODUCE:

1)  Apply patch from bug 305884
2)  Apply patch from bug 307313
3)  Start up with WAY_TOO_MUCH_GC

Assertion failure: c <= cs->length, at ../../../mozilla/js/src/jsregexp.c:1948

(gdb) where
#3  0xb7fd0103 in JS_Assert (s=0xb7ff4e73 "c <= cs->length", 
    file=0xb7ff4880 "../../../mozilla/js/src/jsregexp.c", ln=1948)
    at ../../../mozilla/js/src/jsutil.c:63
#4  0xb7fb7866 in AddCharacterToCharSet (cs=0x81dabd8, c=47103)
    at ../../../mozilla/js/src/jsregexp.c:1948
#5  0xb7fb80cc in ProcessCharSet (gData=0xbfffda10, charSet=0x81dabd8)
    at ../../../mozilla/js/src/jsregexp.c:2150
#6  0xb7fba3d9 in InitMatch (cx=0x82fb138, gData=0xbfffda10, re=0x81dab48)
    at ../../../mozilla/js/src/jsregexp.c:2960
#7  0xb7fba560 in js_ExecuteRegExp (cx=0x82fb138, re=0x81dab48, str=0x8256388, 
    indexp=0xbfffdac8, test=1, rval=0xbfffdc30)
    at ../../../mozilla/js/src/jsregexp.c:3001
#8  0xb7fbc0f5 in regexp_exec_sub (cx=0x82fb138, obj=0x81cbcd8, argc=1,
argv=0x831eeb4, 
    test=1, rval=0xbfffdc30) at ../../../mozilla/js/src/jsregexp.c:3748
#9  0xb7fbc1af in regexp_test (cx=0x82fb138, obj=0x81cbcd8, argc=1, argv=0x831eeb4, 
    rval=0xbfffdc30) at ../../../mozilla/js/src/jsregexp.c:3767
#10 0xb7f73acb in js_Invoke (cx=0x82fb138, argc=1, flags=0)
    at ../../../mozilla/js/src/jsinterp.c:1163
#11 0xb7f834ae in js_Interpret (cx=0x82fb138, pc=0x8203605 ":", result=0xbfffe3cc)
    at ../../../mozilla/js/src/jsinterp.c:3458
#12 0xb7f73b4d in js_Invoke (cx=0x82fb138, argc=0, flags=0)
    at ../../../mozilla/js/src/jsinterp.c:1183
#13 0xb7f834ae in js_Interpret (cx=0x82fb138, pc=0x8288beb ":", result=0xbfffeb9c)
    at ../../../mozilla/js/src/jsinterp.c:3458
#14 0xb7f73b4d in js_Invoke (cx=0x82fb138, argc=1, flags=2)
    at ../../../mozilla/js/src/jsinterp.c:1183
#15 0xb7997e64 in nsXPCWrappedJSClass::CallMethod (this=0x8229418,
wrapper=0x81c5180, 
    methodIndex=3, info=0x820f9c8, nativeParams=0xbfffef30)
    at ../../../../../mozilla/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1339
#16 0xb7990b7b in nsXPCWrappedJS::CallMethod (this=0x81c5180, methodIndex=3, 
    info=0x820f9c8, params=0xbfffef30)

(gdb) jsstack 
[Thread -1258189904 (LWP 21824) exited]
0 [native frame]
1 anonymous(file = [xpconnect wrapped (nsISupports, nsIFile, nsILocalFile) @
0x8221d40 (native @ 0x81b4650)])
["file:///home/bzbarsky/mozilla/vanilla/obj-firefox/dist/bin/components/nsExtensionManager.js":995]
    section = "[e-mail]"
    filePD =
"/home/bzbarsky/mozilla/vanilla/obj-firefox/dist/bin/extensions/[e-mail]"
    this = [object Object]
2 anonymous()
["file:///home/bzbarsky/mozilla/vanilla/obj-firefox/dist/bin/components/nsExtensionManager.js":2541]
    em = [object Object]
    installItem = [function]
    canUse = [function]
    getParamBlock = [function]
    installDroppedInFiles = [function]
    isDirty = false
    ignoreMTimeChanges = false
    newItems = 
    droppedInFiles = 
    xpinstallStrings = 
    installLocations = [object Object]
    location = [object Object]
    actualItems = [object Object]
    entries = [object Object]
    entry = [xpconnect wrapped (nsISupports, nsIFile, nsILocalFile) @ 0x8221d40
(native @ 0x81b4650)]
    installRDF = [xpconnect wrapped nsIFile @ 0x81ef410 (native @ 0x81b5550)]
    id = undefined
    item = undefined
    zipReader = undefined
    prettyName = undefined
    jar = undefined
    principal = undefined
    certPrincipal = undefined
    lf = undefined
    actualMTime = undefined
    i = undefined
    this = [object Object]
3 anonymous(commandLine = [xpconnect wrapped nsICommandLine @ 0x8278c18 (native
@ 0x8270818)])
["file:///home/bzbarsky/mozilla/vanilla/obj-firefox/dist/bin/components/nsExtensionManager.js":2204]
    isDirty = false
    forceAutoReg = false
    componentList = [xpconnect wrapped nsIFile @ 0x81ec688 (native @ 0x81b8cb8)]
    needsRestart = undefined
    this = [object Object]
4 [native frame]

Line 995 of nsExtensionManager.js after preprocessing is:

    if (gIDTest.test(section))

which is
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in&rev=1.149#1000

More details at the assert site:

(gdb) frame 4
#4  0xb7fb7866 in AddCharacterToCharSet (cs=0x81dabd8, c=47103)
    at ../../../mozilla/js/src/jsregexp.c:1948
1948        JS_ASSERT(c <= cs->length);
(gdb) p c
$1 = 47103
(gdb) p cs->length
$2 = 102
(gdb) p *gData->regexp->source
$10 = {length = 136162288, chars = 0x81bef5c}

Looking at jsregexp.c, I see at least the following problems:

regexp_compile() calls JS_ValueToString several times in a row and does nothing
to prevent the previous strings from being GCed.  Not sure whether that's the
issue here...

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

I poked at the code some (I haven't tried to reproduce it yet) and it seems that
the assertion made at the bottom of comment 0 isn't correct, since when we call
js_ValueToString, we immediately store the result in argv (presumably a local
root). The regexp in question is a regexp literal, which means it gets atomized
almost immediately after creation, and the js_NewStringCopyN in
js_NewRegExpObject should be protected by the newborn array.

Boris, is this reproduceable for you? Do you have venkman (or similar running)?

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is 100% reproducible following the steps in comment 0 for me.

I may have venkman compiled in this build, yes... Not sure whether the extension
is installed or anything, of course.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Did a bit of debugging.  We enter js_NewRegExpObject ok, and create str fine,
and create the JSRegExp fine, but the JS_NewObject call ends up GCing the string
somehow.

At the end of js_NewRegExpObject we have:

(gdb) p cx->newborn
$32 = {0x81be4d8, 0x0, 0x0, 0x0, 0x81b1b58, 0x0 <repeats 11 times>}
(gdb) p str
$33 = (JSString *) 0x81be4d8
(gdb) p *str
$34 = {length = 136107744, chars = 0x81b1b5c}

Comment 4

[Boris Zbarsky [:bzbarsky]]

So the problem is that this is the first-ever RegExp object, so we trigger class
lazy init.  And then we get:

#0  JS_ClearNewbornRoots (cx=0x81a6758) at ../../../mozilla/js/src/jsapi.c:1687
#1  0xb795deab in ~XPCCallContext (this=0xbfffdae0)
    at ../../../../../mozilla/js/src/xpconnect/src/xpccallcontext.cpp:348
#2  0xb79a64d2 in XPC_WN_Helper_NewResolve (cx=0x81a6758, obj=0x80ebfa0, 
    idval=135176380, flags=16, objp=0xbfffdc18)
    at ../../../../../mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1080
#3  0xb7f98aba in js_LookupPropertyWithFlags (cx=0x81a6758, obj=0x80ebfa0,
id=135186128, 
    flags=16, objp=0xbfffdc98, propp=0xbfffdc94) at
../../../mozilla/js/src/jsobj.c:2594
#4  0xb7f9714c in js_FindConstructor (cx=0x81a6758, start=0x0, name=0xb7fe98f4
"RegExp", 
    vp=0xbfffdce0) at ../../../mozilla/js/src/jsobj.c:1988
#5  0xb7f9cd15 in GetClassPrototype (cx=0x81a6758, scope=0x0, name=0xb7fe98f4
"RegExp", 
    protop=0xbfffdd68) at ../../../mozilla/js/src/jsobj.c:3719
#6  0xb7f96ab5 in js_NewObject (cx=0x81a6758, clasp=0xb7ffdd80, proto=0x0,
parent=0x0)
    at ../../../mozilla/js/src/jsobj.c:1869
#7  0xb7fbb5ea in js_NewRegExpObject (cx=0x81a6758, ts=0x81bcf60, chars=0x81cd158, 
    length=100, flags=1) at ../../../mozilla/js/src/jsregexp.c:3854

So as things stand, depending on newborns any time we might be ending up in
class init is deadly.

Comment 5

[Brendan Eich [:brendan]]

Crap, lazy standard class init hosed the age-old invariant that a newborn of one
gc-thing type will not be displaced when allocating a gc-thing of another type.

/be

Comment 6

[Brendan Eich [:brendan]]

Attachment: proposed fix

Rather than invent yet another local root stack in which to push cx->newborn so
its pigeon-holes may be overwritten by the lazy standard class resolve API (or
indeed any resolve hook for the global object that tries to allocate when
passed a standard class id), let's just switch to the non-pigeon-hole modern
way.

/be

Comment 7

[Brendan Eich [:brendan]]

The perf hit here will be mitigated completely, and perf increased compared to
what it has been without this bug's patch, by the right fix for bug 304376.

/be

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 195239 [details] [diff] [review]
proposed fix

Maybe I'm being dense, but I don't see how this will fix the bug here. As I
understand things, problem isn't necessarily that the class instantiation
knocks our newborn string out of cx->newborn, but that XPConnect itself is
clearing the newborn table (comment 4). I cannot reproduce this bug in the
standard js shell (with WAY_TOO_MUCH_GC), I think for this reason. Am I missing
something, or is this not completely sufficient (I think this patch is a good
idea, anyway, though).

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 195239 [details] [diff] [review]
proposed fix

sr=shaver

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #8)
> (From update of attachment 195239 [details] [diff] [review] [edit])
> XPConnect itself is clearing the newborn table (comment 4).

That is a bug, isn't it?  I'll dig into cvs blame and see about another patch.

/be

Comment 11

[Brendan Eich [:brendan]]

Ok, I'm to blame -- bryner's checkin of rev 1.12 of xpccallcontext.cpp
identifies bug 198655, in which I'm named as the Godfather (or maybe 'Fredo ;-).

Now I'll have to dig into why it seemed necessary to clear newborn roots in the
dtor for an XPConnect call context to prevent leaks to-do with JS components. 
Obvious theory is that we were never clearing on return from outermost call from
native into JS via XPConnect.  That suggests a fix where we clear newborn roots
only if !cx->fp.

/be

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 195239 [details] [diff] [review]
proposed fix

Ok, I guess we should do this anyway. r=mrbkap

Comment 13

[Brendan Eich [:brendan]]

Attachment: try to fix xpconnect

bz, can you try these two patches and see how much further your WAY_TOO_MUCH_GC
build gets?  Thanks,

/be

Comment 14

[Brendan Eich [:brendan]]

Attachment: working xpconnect patch

Thanks to bz for fixing the bogo-patch, and testing both patches.  They seem to
fix this bug, but we still have miles to go before WAY_TOO_MUCH_GC sleeps.

/be

Comment 15

[Brendan Eich [:brendan]]

Comment on attachment 195239 [details] [diff] [review]
proposed fix

Pondering wrapping all of GetClassPrototype in a local root scope, instead of
doing this in js_FindConstructor.  If you look at GetClassPrototype, after it
call js_FindConstructor, it checks whether the class named a function, and if
so gets its prototype property's value.  That will not cause any more
allocations in the lazy class init case (because the class prototype is created
and populated by this point), but in general it's another resolve hook out-call
that could do whatever.

Another alternative is to local-root-scope all resolve hook calls, but that
seems likely to hurt performance, and it covers way too many cases that don't
need protection because callers are not counting on newborn roots.

The newborn root model is too just-so and "local" to scale up.	At least we
have JNI-style local root stacks now.

Anyway, thoughts welcome.

/be

Comment 16

[Brendan Eich [:brendan]]

Attachment: fix for another bug bz hit

on his way to WAY_TOO_MUCH_GC nirvana.

/be

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 195307 [details] [diff] [review]
working xpconnect patch

sr=bzbarsky.

With these three patches I get all the way up to SetNewDocument on a window
before I crash!  I'll file more bugs on things as I debug them enough.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 195313 [details] [diff] [review]
fix for another bug bz hit

r=mrbkap

Comment 19

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 195313 [details] [diff] [review]
fix for another bug bz hit

sr=shaver

Comment 20

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 195307 [details] [diff] [review]
working xpconnect patch

!! r=shaver

Comment 21

[Brendan Eich [:brendan]]

Fixed on the trunk, gonna leave open to catch similar bugs, and to bake for the
1.8 branch.

/be

Comment 22

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch suggested by brendan

So I crashed closing the large version of the airplane emergency landing image
currently on http://www.nytimes.com/ , while brendan was nearby, and he and
mrbkap debugged.  They came up with this fix.  The crash stack was:

#5  <signal handler called>
#6  js_NewGCThing (cx=0xb4097c8, flags=4, nbytes=28)
    at /builds/trunk/mozilla/js/src/jsgc.c:586
#7  0xb7be5ada in AllocSlots (cx=0xb4097c8, slots=0xb0cd8a4, nslots=6)
    at /builds/trunk/mozilla/js/src/jsobj.c:1810
#8  0xb7bebf99 in js_SetRequiredSlot (cx=0xb4097c8, obj=0xad8fea0, slot=5,
    v=162084048) at /builds/trunk/mozilla/js/src/jsobj.c:4211
#9  0xb7b9fc9d in JS_SetReservedSlot (cx=0xb4097c8, obj=0xad8fea0, index=1,
    v=163085921) at /builds/trunk/mozilla/js/src/jsapi.c:3187
#10 0xb79c7f57 in XPCNativeMember::Resolve (this=0x9b87e60, ccx=@0xbfeaa490,
    iface=0x9b87b79)
    at /builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativeinfo.cpp:189
#11 0xb79a040d in XPCNativeMember::GetValue (this=0x9b87e60, ccx=@0xbfeaa490,
    iface=0x9b87b78, pval=0x9a934d0) at xpcprivate.h:1113
#12 0xb79ccaac in DefinePropertyIfFound (ccx=@0xbfeaa490, obj=0xb45b520,
    idval=162346204, set=0x9b36880, iface=0x9b87b78, member=0x9b87e60,
    scope=0xb4114d0, reflectToStringAndToSource=1,
    wrapperToReflectInterfaceNames=0x0, wrapperToReflectDoubleWrap=0x0,
    scriptableInfo=0xb1059d0, propFlags=1, resolved=0x0)
    at /builds/trunk/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp:447

...

(gdb) frame 6
#6  js_NewGCThing (cx=0xb4097c8, flags=4, nbytes=28)
    at /builds/trunk/mozilla/js/src/jsgc.c:586
586		*flp = thing->next;
Current language:  auto; currently c
(gdb) p thing
$1 = (JSGCThing *) 0x4
(gdb) p rt->gcFreeList
$2 = {0xad8fed0, 0x0, 0xb0cd8b8, 0x4, 0x9c5fa70, 0xa4c9c70, 0x9ae3448,
  0x9ae0a00, 0x0, 0x9ae46c0}

(sizeof(JSFunction) == 32)


(gdb) p cx->newborn
$11 = {0xad8fe50, 0xad8fbe8, 0x0, 0x0, 0x9c5f2c0, 0x0 <repeats 11 times>}


in frame 9:
(gdb) p obj
$4 = (JSObject *) 0xad8fea0
(gdb) p obj->slots
$5 = (jsval *) 0xb0cd8a4
(gdb) p/x obj->slots[3] - 1
$7 = 0xbaa7940
(gdb) p (JSFunction*)$
$8 = (struct JSFunction *) 0xbaa7940
(gdb) p *$
$9 = {nrefs = 1, object = 0xad8fea0, u = {
    native = 0xb79cb254 <XPC_WN_CallMethod(JSContext*, JSObject*, unsigned int,
long*, long*)>, script = 0xb79cb254}, nargs = 0, extra = 0, nvars = 0,
  flags = 0 '\0', interpreted = 0 '\0', nregexps = 0, spare = 0,
  atom = 0x9b13c48, clasp = 0x0}


vim modeline added per mrbkap's request

Comment 23

[Asa Dotzler [:asa]]

Time is getting really short for the branch. If we have something for the
branch, let's get it in ASAP.

Comment 24

[Scott MacGregor]

Another ping. The last patch posted to this bug has been sitting for a week now.
This bug is in danger of not making 1.5 if we can't get some traction on it
soon. Thanks!

the patches that have been baking have been doing so since 09/14, are they ready
to land yet? 

Comment 25

[Brendan Eich [:brendan]]

See the newly noted dependency.

/be

Comment 26

[Blake Kaplan (:mrbkap) (inactive)]

I just backed out bug 195239 to seee if it's the cause of bug 308678.

Comment 27

[Blake Kaplan (:mrbkap) (inactive)]

That's attachment 195239 [details] [diff] [review], even.

Comment 28

[Asa Dotzler [:asa]]

we'd consider approving safe and effective patches so if you get that, request
approval. thanks.

Comment 29

[Boris Zbarsky [:bzbarsky]]

So... it's possible that that backout helped bug 310456.

Comment 30

[Bill Gianopoulos [:WG9s]]

(In reply to comment #27)
> That's attachment 195239 [details] [diff] [review] [edit], even.

So far this seems to have done the trick.  Assuming this actually cures the
crashes, is the issue that this was just a bad idea?  Or, does the original
behavior need to be reverted for the case where the call to
js_EnterLocalRootScope fails?

Comment 31

[Brendan Eich [:brendan]]

(In reply to comment #30)
> (In reply to comment #27)
> > That's attachment 195239 [details] [diff] [review] [edit] [edit], even.
> 
> So far this seems to have done the trick.  Assuming this actually cures the
> crashes, is the issue that this was just a bad idea?  Or, does the original
> behavior need to be reverted for the case where the call to
> js_EnterLocalRootScope fails?

Failure is failure, there's nothing to do but propagate it -- otherwise a nested
last-ditch GC could collect a newborn.

But is failure even happening in the real world?  Or is the problem here that
this patch increased the frequency of a latent bug?  That's what it looks like
to me.

/be

Comment 32

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 195307 [details] [diff] [review]
working xpconnect patch

This patch was never checked in on the branch, and it looks like that is hurting some embedding customers badly (and thus potentially hurting us). The potential risk of taking this is that it might hold on to some objects longer than before.

This patch has been baking on the trunk for quite a while (since early September) with no apparent ill effects.

Comment 33

[Asa Dotzler [:asa]]

Comment on attachment 195307 [details] [diff] [review]
working xpconnect patch

sorry, too late.

Comment 34

[timeless]

*** Bug 314989 has been marked as a duplicate of this bug. ***

Comment 35

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 195307 [details] [diff] [review]
working xpconnect patch

This fix is safe and has been baking on the trunk for quite a while. The rest of the patches on this bug are seperate and can be ignored. This patch fixes potential crash bugs in the JS engine depending on when GC runs. We should move the patch that was backed out into another bug to see if we can track it down the problem that it caused.

Comment 36

[Boris Zbarsky [:bzbarsky]]

*** Bug 324043 has been marked as a duplicate of this bug. ***

Comment 37

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 195313 [details] [diff] [review]
fix for another bug bz hit

Brendan, is this safe for 1.8 branch in general?  Might be worth it.  Not sure about 1.8.0 branch...

Comment 38

[Brendan Eich [:brendan]]

The attachment 195313 [details] [diff] [review] is safe for any branch.  It should've gone into 1.8.0.1.

/be

Comment 39

[Brendan Eich [:brendan]]

Comment on attachment 195313 [details] [diff] [review]
fix for another bug bz hit

Sure, let's get this into the 1.8 branch soonest. I'm going to approve for 1.8.0.2 also, since the patch is well-baked and a proven fix for topcrash-y bugs.

/be

Comment 40

[timeless]

*** Bug 319168 has been marked as a duplicate of this bug. ***

Comment 41

[Ty Williams]

The bug I've been experiencing has been marked a dupe of this bug.  Here's the Talkback for today's crash TB14624879M.

Comment 42

[Jesus Cea]

Hope this bug been resolved in 1.8.0 branch. I'm having dozens of crashes by day, using sessionsaver. In fact I usually have a difficult time restarting the browser, since I have tons of windows/tabs. Continuous crashes, you know.

It's good I keep backups of my prefs.js :-).

Example of crash: TB14669288M

Comment 43

[Ty Williams]

Still crashing after upgrade to Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

Talkback ID for this crash: TB14931490Y

Comment 44

[Jesus Cea]

Judging by my continuous crashes, seems that the fix haven't landed in the 1.8.0 branch, yet.

I'm currently using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.1) Gecko/20060208 Firefox/1.5.0.1 ID:2006020804, and I needed EIGHT retries to sucessfully launch my current firefox session (with session saver extension). Yesterday I needed 7, 12, 7, 2, 4, 6, 11, 5, 12 and 9 retries. Yes, it crashed ten times yesterday. Relaunch crashes a lot, because I use SessionSaver with about 33 windows open, each window with perhaps five tabs.

Waiting for the bug fix landing as a god bless...

Comment 45

[Boris Zbarsky [:bzbarsky]]

When this lands on branch, the relevant keyword will be set (fixed1.8.0.2, to be exact).  Until then, PLEASE stop generating bugmail on this bug.  Or have the kindness to remove me from the cc list before the next round.

Comment 46

[Daniel Veditz [:dveditz]]

Comment on attachment 195307 [details] [diff] [review]
working xpconnect patch

approving for 1.8.0 branch, a=dveditz for drivers

Comment 47

[Brendan Eich [:brendan]]

*** Bug 327871 has been marked as a duplicate of this bug. ***

Comment 48

[Ty Williams]

Had a trifecta of crashes on the 16 that I think are all this bug. TB15270336Z TB15266963W TB15248368G. Also had a crash this mornining that I think is this bug. TB15340505Q.

Comment 49

[Marc Bejarano]

the severity of this bug should be critical, no?

Comment 50

[Bill Gianopoulos [:WG9s]]

(In reply to comment #48)
> Had a trifecta of crashes on the 16 that I think are all this bug. TB15270336Z
> TB15266963W TB15248368G. Also had a crash this mornining that I think is this
> bug. TB15340505Q.
> 

Perhaps I am just stupid, but I fail to see what ANY of the talkcback reports you keep posting about here have to do with this bug.

Comment 51

[Bill Gianopoulos [:WG9s]]

(In reply to comment #50)
> (In reply to comment #48)
> > Had a trifecta of crashes on the 16 that I think are all this bug. TB15270336Z
> > TB15266963W TB15248368G. Also had a crash this mornining that I think is this
> > bug. TB15340505Q.
> > 
> 
> Perhaps I am just stupid, but I fail to see what ANY of the talkcback reports
> you keep posting about here have to do with this bug.

Nevermind.  I am stupid.  Just noticed that Brendan duped the crash bug to this one.  Sorry for the SPAM.

Comment 52

[Brendan Eich [:brendan]]

*** Bug 325945 has been marked as a duplicate of this bug. ***

Comment 53

[Jay Patel [:jay]]

Looks like dveditz gave the approval for 1.8.0.2 on 2/14, so hopefully we get this in on that branch before it's too late.   Who plans to check this patch in?  Brendan/Blake?

Just want to make sure this doesn't miss the train, since it will likely fix A LOT of topcrashers we've been seeing in Firefox 1.5.x. 

Comment 54

[Robin Monks]

Add TB15304243 (and possibily TB15306578 ) to the parade.

Robin

Comment 55

[Boris Zbarsky [:bzbarsky]]

PLEASE STOP SPAMMING THIS BUG.  See comment 45 and such.  There's really absolutely no reason to be posting talkback IDs here, or any comments other than those related to landing this on the branches and verifying the fix.  Zero.  None.  Zilch.  Please stop doing it.

Comment 56

[Ty Williams]

Mr. Zbarsky,

I am just a user. I was under the impression that when I found a problem with one of the Mozilla products, I was supposed to file bug reports.  Are you saying it is the Mozilla orginization's policy for users to not report and follow up on bugs? If so, I am sorry for trying to be helpful.

Ty

Comment 57

[Brendan Eich [:brendan]]

(In reply to comment #56)
> Mr. Zbarsky,
> 
> I am just a user. I was under the impression that when I found a problem with
> one of the Mozilla products, I was supposed to file bug reports.  Are you
> saying it is the Mozilla orginization's policy for users to not report and
> follow up on bugs?

No, he's not.  He's asking you to read prior comments and respect them, and in this bug's case, comment 45 means no need to keep saying "me too".  This is in fact Mozilla "bug etiquette" policy.  See https://bugzilla.mozilla.org/page.cgi?id=etiquette.html point 1.1.

> If so, I am sorry for trying to be helpful.

No problem.  Don't take it personally -- bugs are high-volume tools for hackers and if too many "me toos" or other more user-forum-like comments creep in, they become hard to use for hackers who might actually help fix them, which defeats the purpose. ;-).

/be

Comment 58

[Brendan Eich [:brendan]]

"working xpconnect patch" committed to the 1.8 and 1.8.0 branches.

"fix for another bug bz hit" patch is superceded by the patch for bug 325269, which I just landed on the 1.8* branches with approval.

/be

Comment 59

[Boris Zbarsky [:bzbarsky]]

Mr. Williams, reporting bugs is very much desirable and I very much appreciate your doing it.  All I'm asking for is that when adding comments to an existing bug one consider whether the comments will help get the bug fixed.  If you're seeing crashes that you think are due to an existing bug, I'd file a new bug and mark it dependent on the existing bug, myself...

I definitely didn't mean to make it sound like we don't want user feedback.  We do.

Comment 60

[Brendan Eich [:brendan]]

*** Bug 328366 has been marked as a duplicate of this bug. ***

Comment 61

[Dave Liebreich [:davel]]

Please provide guidance for testing this fix.

Comment 62

[timeless]

*** Bug 321610 has been marked as a duplicate of this bug. ***