351773 - Frequent crashes involving JS GC in recent trunk builds 2006-09

Status: RESOLVED INCOMPLETE

(Core :: XUL, defect)

Description

[Adam Guthrie]

I hate to file a bug like this, but I can't seem to use a trunk build from 2006-09-07-04 for more than 5 minutes without crashing.

I'm 100% that this regressed sometime between 2006-09-06-04 and 2006-09-07-04, as I wasn't crashing (at all) in yesterday's build.

Check-ins: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-09-06+03&maxdate=2006-09-07+05&cvsroot=%2Fcvsroot

Here's a couple of Talkback IDs for the crashes: TB23031356G, TB23016701X.

Comment 1

[Adam Guthrie]

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

Comment 2

[Adam Guthrie]

Brendan and I did some remote gdbing on this. It turns out that it's related to XUL popups in some way or another.

The filename of the "script" we're running is "chrome://global/content/bindings/popup.xml" and we're on line #228 of that which is here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/content/widgets/popup.xml&rev=1.10#228. More debugging showed that that was being called from an "onxblpopuphiding" event (?).

Neil, are you seeing anything like this in builds with the patch for bug 70798 in them?

Comment 3

[Adam Guthrie]

Neil's patch in bug 351716 seems to fix this. It would be nice to figure out the underlying issue, though (if there is one...).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Well.  One obvious issue I see is that in nsMenuFrame::OnDestroy we dispatch an event to mContent without any provisions for keeping it alive while the event is going.  Unless someone else handles that, it looks to me like mContent could just die in the middle of that event dispatch.  Not sure that would manifest itself as GC issues.

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #4)
> Well.  One obvious issue I see is that in nsMenuFrame::OnDestroy we dispatch an
> event to mContent without any provisions for keeping it alive while the event
> is going.  Unless someone else handles that, it looks to me like mContent could
> just die in the middle of that event dispatch.  Not sure that would manifest
> itself as GC issues.

It wasn't clear why JS_GetPrivate was crashing -- in gdb, as far as ispiked and I could see, obj and its private data and class all looked fine.

Anyway, can the event hold its content node without making a cycle?  (The cycle colelctor is coming, so we shouldn't fail to use a strong ref if it's needed out of fear of cyclic leaks.)

/be

Comment 6

[Boris Zbarsky [:bzbarsky]]

> Anyway, can the event hold its content node without making a cycle?

Hmm... actually, the event might hold such a ref in mTarget anyway...

If that were actually the issue, event dispach is sync, so just holding a stack comptr at the dispatch site would work.

Comment 7

[Adam Guthrie]

Bug 351716 fixed this crash, but I guess we'll leave this open for the issue Boris brought up.

Comment 8

[Brendan Eich [:brendan]]

(In reply to comment #7)
> Bug 351716 fixed this crash, but I guess we'll leave this open for the issue
> Boris brought up.

Really, for the underlying bug that bug 351716 works around, I would say.  No XBL change should be able to crash us this way.

/be

Comment 9

[Boris Zbarsky [:bzbarsky]]

I'm not sure that problem was actually a problem.  As in, we still don't know what was really up here.

Comment 10

[Wayne Mery (:wsmwk)]

bz  comment #6)
> Hmm... actually, the event might hold such a ref in mTarget anyway...
> 
> If that were actually the issue, event dispach is sync, so just holding a stack
> comptr at the dispatch site would work.

what would be a reasonable new summary for this bug for the above task, or should a new bug be filed?

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #10)
> bz  comment #6)
> > Hmm... actually, the event might hold such a ref in mTarget anyway...
> > 
The whole event target chain is kept alive during dom event dispatch.

Comment 12

[Jesse Ruderman]

Sounds like bug 351716 fixed the crash, and the underlying problem (if any) was never determined.