140088 - Chatzilla is crashing when switching tabs.

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Frederic Bezies]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:0.9.9+)
Gecko/20020425
BuildID:    2002042503

All is in the summary. Cannot switch tabs without getting a crash :-/

Sent Talkback ID : TB5607939Q

Reproducible: Always
Steps to Reproduce:
1.Open chatzilla.
2.Connect to a network and a channel.
3.switch tabs, for example from irc to *client*

Actual Results:  crash

Expected Results:  switching view

downloading talkback build from trunk.

Comment 1

[Robert Ginda]

Attachment: talkback

Comment 2

[Olivier Cahagne]

couldn't find dupes, marking NEW.

btw, it wfm using build 20020424 on Win2k on irc.mozilla.org +  #mozillazine and
#kill-unco.

Comment 3

[Robert Ginda]

I just crashed with the same stack, talkback ID 5749291.

Comment 4

[Robert Ginda]

jst, I think this is related to one of your patches.  Can you have a look?

Comment 5

[Robert Ginda]

This might have something to do with heikki's checkin (for jst) for bug 138138
(revision 1.29 of nsTreeBoxObject.cpp), but I'm just guessing.

There is garbage in the hash table, either we put garbage there, or (more
likely) we put something there but didn't get the reference counting right
(we're crashing tring to addref in nsSupportsHashtable.)

This crash exists on the 1.0 branch, and makes chatzilla quite unusable.  I
think I ran into it changing tabs in composer as well, but I don't have a stack.

Hewitt, are you out there?

Comment 6

[Johnny Stenback (:jst)]

If this is a branch problem then this is completely unrelated to my changes
since none of my changes ever touched the branch, I only checked those and other
related fixes in on the trunk *after* the branch was created.

Comment 7

[Robert Ginda]

I just reproduced this with today's build,
<http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=5871143>

Comment 8

[Robert Ginda]

d'oh, that came from the latest/ directory, which is probably not the branch. 
I'll try again.

Comment 9

[Robert Ginda]

<http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=5874253>

There it is on the branch, too.

Comment 10

[Johnny Stenback (:jst)]

Looks like the treebody frame is being destroyed somewhere w/o the
nsTreeBoxObject being notified about the destruction of the treebody frame. This
is similar to what happened in bug 138138, but I don't know off hand what
triggers this situation here. In 138138 the presentation was torn down due to
the document viewer being hidden but the xul document wasn't notified about this
so its boxobject hash stayed around holding on to presentation data (i.e.
nsIFrame's) past the destruction of the presentation data. This seems related,
but also different...

Comment 11

[Robert Ginda]

I've got a tree in a container with a visibility="false" attribute (I keep my
userlist hidden.)  I wonder if that is related.

Comment 12

[Robert Ginda]

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

Comment 13

[Johnny Stenback (:jst)]

Attachment: Oops, wrong diff.

Comment 14

[Johnny Stenback (:jst)]

Comment on attachment 82264 [details] [diff] [review]
Oops, wrong diff.

Oops, wrong diff, new one coming up...

Comment 15

[Johnny Stenback (:jst)]

Attachment: Possible fix, clear the box object for elements that are removed from a XUL document

Comment 16

[Robert Ginda]

jst's patch seems to cure the crash for me.

Comment 17

[Johnny Stenback (:jst)]

Woohoo!

Comment 18

[David Hyatt]

This is supposed to happen in nsXULElement and nsGenericElement 
when you remove an element from a document.  Any idea why these 
aren't being hit for this particular element, jst?

Comment 19

[David Hyatt]

r/sr=hyatt, although I'm perplexed why the SetDocument call doesn't 
handle this.

Comment 20

[Johnny Stenback (:jst)]

Hmm, no, no idea why the code in SetDocument() wouldn't be hit in this case. I
was never able to see this crash in the debugger, so I'm clueless as to why the
current cleanup code doesn't do the trick. Maybe something is failing somewhere
and causing us to not clear the document pointer for the treechildren element,
or something...

Comment 21

[Asa Dotzler [:asa]]

Can't ship a chatzilla that's crashing this consistently. Adding to the make RC2
not suck bug. 

Comment 22

[Robert Ginda]

Comment on attachment 82265 [details] [diff] [review]
Possible fix, clear the box object for elements that are removed from a XUL document

r=rginda
sr=hyatt (from previous comment)

Comment 23

[Asa Dotzler [:asa]]

Comment on attachment 82265 [details] [diff] [review]
Possible fix, clear the box object for elements that are removed from a XUL document

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 24

[Robert Ginda]

checked in to branch only.

Comment 25

[Robert Ginda]

still seeing this on the branch, though less frequently.

http://climate/reports/SingleIncidentInfo.cfm?dynamicBBID=6082902

Comment 26

[Robert Ginda]

Attachment: chatzilla hack

This *avoids* the problem by not changing the tree root when the user list
isn't visible.

This is 1.0 spackle, the real problem still needs to be treated.

Comment 27

[Christopher Blizzard (:blizzard)]

Comment on attachment 82879 [details] [diff] [review]
chatzilla hack

rs=blizzard

Comment 28

[Asa Dotzler [:asa]]

Comment on attachment 82879 [details] [diff] [review]
chatzilla hack

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 29

[Robert Ginda]

fixed on branch

Comment 30

[Robert Ginda]

Attachment: more chatzilla hacking

Forgot to keep from touching the tree selection while the tree was invisible.

Comment 31

[Joe Hewitt (gone)]

I've been trying to reproduce this for a few days, and I can't, so I'm about to
give up on that strategy.  I'll have to rely on what I know in theory, which is
that something is making a call to a tree box object property AFTER it's
treebodyframe has been destroyed, but BEFORE a new one is created, and this
presumably happens sometime DURING a template rebuild.

Comment 32

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

Comment on attachment 83428 [details] [diff] [review]
more chatzilla hacking

sr=shaver with the spelling fixes.

Comment 33

[Samuel Sieb]

Comment on attachment 83428 [details] [diff] [review]
more chatzilla hacking

r=[e-mail]
it appears that that section could be just removed completely now.

Comment 34

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 83428 [details] [diff] [review]
more chatzilla hacking

a=[e-mail] for checkin.  Make sure it's in the branch and trunk

Comment 35

[Robert Ginda]

workarounds checked in to branch and trunk

Comment 36

[Cryo]

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

Comment 37

[Lee Jonas]

Attachment: possible fix - suppresses select events after tree is destroyed

I cannot reproduce the error, but this is a fix to a similar problem found in
my own XUL app.

The handling of a tree select event by the tree selection object involves
dereferencing a weak reference to the tree.  If the select event is issued just
before a call to 'Destroy()' the tree, it may get handled after the tree is
gone.  The pointer it dereferences is now a dangling pointer.  The result is an
access violation.

The fix is a call within the tree's 'Destroy()' method to suppress further
handling of select events.

Comment 38

[Lee Jonas]

Comment on attachment 117511 [details] [diff] [review]
possible fix - suppresses select events after tree is destroyed

This prevents delayed handling of selection events from using a weak reference
to the tree body as it is being destroyed.

Comment 39

[Jan Varga [:janv]]

Could you check if it still crashes ?
Neil checked in a patch recently which fixed a similar crash in Firebird.

Comment 40

[Frederic Bezies]

Sorry, but I am not using Chatzilla right now, it seems like I am banned (??)
from mozilla.org IRC server.

Comment 41

[Frederic Bezies]

Two news : I am not anymore under windows XP, so I cannot check if this bug
still appears. At least, linux is not concerned by it.

And trying with a 3 hours old CVS based build, tab switching is working great.

Comment 42

[Jan Varga [:janv]]

Cool, can somebody check on winxp ?
It shouldn't crash anymore.

Comment 43

[Frederic Bezies]

It works since this fix. I get back under XP.

Time to close this bug as fixed, isn't it ? ;)

Comment 44

[Jan Varga [:janv]]

yes, it is

Comment 45

[Frederic Bezies]

Marking bug as fixed, since this bug is dead for a long time ;o)

Reopen it if I had made a mistake ;o)