196446 - Crash in [@ nsXBLPrototypeBinding::BindingDetached] [@ nsXBLPrototypeHandler::ExecuteHandler] [@ nsXBLPrototypeBinding::GetBindingElement][@ nsXBLPrototypeBinding::GetRuleProcessors] [@ nsXBLBinding::GetImmediateChild]

Status: VERIFIED FIXED

(Core :: XBL, defect)

Description

[tenthumbs]

I'm starting this off in XBL because that's where the crash is.

With a fresh trunk build from this morning using gcc 3.2.2 I get a crash the
first time I start mozilla. I can kill it and get past the problem. Well,
actually I see bug 194614 so it takes two more tries to get a functioning
mozilla. I tend to think this is related to that bug.

I can reproduce the crash by removing XUL.mfasl and compreg.dat. Mozilla dies
the next time it starts.

Comment 1

[tenthumbs]

Attachment: session log

Session log with stack trace.

Comment 2

[tenthumbs]

Upping severity.

Comment 3

[tenthumbs]

Attachment: session log in a fresh account

I hid ~/.mozilla and removed compreg.dat and xpti.dat so it should be close to
a fresh start. The stack trace is a little different.

Comment 4

[tenthumbs]

Forgot to mention that in attachment 116635 [details] mozilla cannot reach
<http://www.mozilla.org/start> because it's not yet using my proxy server.

Comment 5

[tenthumbs]

Attachment: another session log

I edited /etc/hosts so mozilla can find www.mozilla.org (actually my Apache
server) and the stack trace is different a third way.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

I saw this too, both in a nightly and in my own build. Running mozilla with "-P
profilename" seems to fix it for me.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

actually, my bug seems to be different, it crashes every time...

Comment 8

[tenthumbs]

Maybe it's the state of XUL.mfasl. Does your crash change if you remove
compreg.dat and XUL.mfasl? I should be clear that I can always reproduce this if
I remove those files.

Comment 9

[Nicholas Allen]

That first stack trace showed up in a bug yesterday.  I believe it was from
deCOMtamination of XBL...

Comment 10

[tenthumbs]

The last good build I had was updated on Thu Mar 6 07:56:08 EST 2003. The XBL
changes seem to be from bug 194836 but I don't see any crashes there.

Comment 11

[tenthumbs]

Make that bug 194834.

Comment 12

[tenthumbs]

Ah, bug 196337 comment #3 is similar to my first one but I removed
  user_pref("nglayout.debug.invalidate_dumping", false);
and it still crashes.

Comment 13

[tenthumbs]

Gack, I cannot do anything today! I actually removed 
   user_pref("nglayout.debug.disable_xul_cache", false);

Comment 14

[John Morrison]

Actually, I think this is the same as bug 196337 (and also bug 196477)

-> bryner

Comment 15

[Boris Zbarsky [:bzbarsky]]

See also comments in bug 194834 that have a stack showing that we end up with a
garbage "this" in nsXBLPrototypeHandler::ExecuteHandler.

Comment 16

[Andreas Kunz]

Isn't this a topcrash+?
ftp://ftp.mozilla.org/pub/data/crash-data/Trunk-topcrashers.html seems to
suggest so - actually it is now on position 1 amongst the open crashers!
It also seems to suggest that this crashes on all/all.

Comment 17

[Jan Carpenter]

Marking the subject line so this bug will show up on the topcrash reports
associated with this stack sig.

Comment 18

[Jay Patel [:jay]]

Making this topcrash+ and adding regression keyword.  This crash (and about 4
other XBL related crashes) started with MozillaTrunk builds on 3/7.  If we can
find the checkin that caused this, we can make this a zt4newcrash.

Comment 19

[Jay Patel [:jay]]

Making this zt4newcrash since it looks like Bryner's changes on 3/6 caused this
regression.  I think he's still working on it (bug 194834)...

Comment 20

[Jay Patel [:jay]]

Adding [@ nsXBLPrototypeHandler::ExecuteHandler] [@
nsXBLPrototypeBinding::GetBindingElement] to track those crashes in this bug as
well.

Comment 21

[Matthias Versen [:Matti]]

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

Comment 22

[Brian Ryner (not reading)]

Attachment: refcount the prototype bindings again

I really didn't want to have to do this, but the alternative (storing the
docinfo in each bound document's binding manager table, in addition to the XUL
cache) added about 200KB of runtime memory usage.

Comment 23

[jag (Peter Annema)]

Comment on attachment 116896 [details] [diff] [review]
refcount the prototype bindings again

sr=jag

Comment 24

[John Keiser (jkeiser)]

Comment on attachment 116896 [details] [diff] [review]
refcount the prototype bindings again

You need to return mRefCnt from Release(), and I'm wondering about this whole
mRefCnt = 1 thing in there ... what's that about?  It seems designed to catch
race conditions, but there are other race conditions in that method that could
simply miss the delete altogether.

Other than that this looks perfectly fine.

Comment 25

[Brian Ryner (not reading)]

Attachment: revised patch

Good call on both counts.  The mRefCnt = 1 was copied from nsISupportsImpl.h,
but it turns out that it's only needed for nsISupports-derived classes.

Comment 26

[Brian Ryner (not reading)]

I checked in a patch that should fix these crashers (from bug 196446).  Please
reopen if they still happen starting with 3/12 builds.

Comment 27

[jag (Peter Annema)]

Good catch on the return mRefCnt.

+  nsrefcnt Release() {
+    --mRefCnt;
+    NS_LOG_RELEASE(this, mRefCnt, "nsXBLPrototypeBinding");
+    if (mRefCnt == 0) {
+      delete this;
+      return 0;
+    }
+    return mRefCnt;
+  }

You can skip the |return 0| now though.

Comment 28

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

No, the |return 0;| is needed to avoid accessing |mRefCnt| after the destructor
has run.

Also, this seems to have broken OS/2 and some (old?) gcc versions.  The gcc
error is:

../../../dist/include/xpcom/nsCOMPtr.h: In instantiation of
`nsDerivedSafe<nsXBLPrototypeBinding>':
nsXBLBinding.cpp:350:   instantiated from here
../../../dist/include/xpcom/nsCOMPtr.h:213: base `nsXBLPrototypeBinding' with
only non-default constructor
../../../dist/include/xpcom/nsCOMPtr.h:213: in class without a constructor

and the OS/2 error is similar.  Some possibilities:
 * remove nsDerivedSafe from nsAutoPtr.h
 * previous, but only do it based on an autoconf test that detects this compiler
bug (it's a bug, right?)
 * something else??

Comment 29

[Adam Hauner]

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

Comment 30

[Boris 'pi' Piwinger]

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

Comment 31

[Jay Patel [:jay]]

v.fixed.  these crashes are gone after 3/11 builds.

Comment 32

[Jay Patel [:jay]]

Added [@ nsXBLPrototypeBinding::GetRuleProcessors] [@
nsXBLBinding::GetImmediateChild] to summary for future reference.  There were
crashes under those stack signatures for this crash also.