215250 - crash in nsTimerImpl::InitWithCallback

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[R.K.Aa.]

Going to http://www.bilnorge.no now causes a crash.
May be related to checkin for bug 215163, see stack attached in attachment 129277 [details]

Comment 1

[R.K.Aa.]

Another way to achieve a crash in nsTimerImpl::InitWithCallback
is to load the bugzilla query page, simply select component "browser" and then
wheelscroll the box. Immediate crash. This may  be a blocker.

#0  0x409169fd in nsTimerImpl::InitWithCallback(nsITimerCallback*, unsigned,
unsigned) ()
#1  0x40bc634d in nsSelectUpdateTimer::Start(nsIPresContext*) ()
#2  0x40bc552b in nsListControlFrame::StartUpdateTimer(nsIPresContext*) ()
#3  0x40bc2d84 in nsListControlFrame::RemoveOption(nsIPresContext*, int) ()
#4  0x40d6ffef in nsHTMLSelectElement::RemoveOptionsFromList(nsIContent*, int,
int) ()
#5  0x40d7052f in nsHTMLSelectElement::WillRemoveOptions(nsIContent*, int) ()
#6  0x40d6fd0e in nsHTMLSelectElement::RemoveChildAt(int, int) ()
#7  0x40ca80b2 in nsGenericElement::doRemoveChild(nsIDOMNode*, nsIDOMNode**) ()
#8  0x40d74c6d in nsHTMLSelectElement::RemoveChild(nsIDOMNode*, nsIDOMNode**) ()
#9  0x40d70a1d in nsHTMLSelectElement::Remove(int) () 
#10 0x40d70bb5 in nsHTMLSelectElement::SetLength(unsigned) ()
#11 0x40d73941 in nsHTMLOptionCollection::SetLength(unsigned) ()

Comment 2

[R.K.Aa.]

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

Comment 3

[Olivier Cahagne]

raising blocking 1.5b flag as it prevents using Mozilla 2003080605 to triage bug
reports on bugzilla.

Comment 4

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

That's because the code in question explicitly works around the timer bug fixed
in  bug 215163 by doing an NS_RELEASE_THIS() before reinitializing the timer.

Comment 5

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

Attachment: patch

This should fix it, although I haven't tested yet.

Comment 6

[Brendan Eich [:brendan]]

Any more such workarounds that need to be removed?

/be

Comment 7

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

Comment on attachment 129296 [details] [diff] [review]
patch

I've tested that this does fix the crash (using steps to reproduce that don't
involve wheel scrolling, since I don't have a wheel mouse).

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 129296 [details] [diff] [review]
patch

r+sr=brendan.

/be

Comment 9

[Asa Dotzler [:asa]]

Comment on attachment 129296 [details] [diff] [review]
patch

a=asa (on behalf of drivers) for checkin to 1.5beta.

Comment 10

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

Fix checked in to trunk, 2003-08-06 17:00 -0700.

Comment 11

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

I checked all the callers of InitWithCallback:
 * I think the original change fixed leaks in
    * nsComposerCommandsUpdater::PrimeUpdateTimer (the original bug)
    * nsNSSComponent::DefineNextTimer
    * nsAutoScrollTimer::Start

Those were the only places (other than the case in this bug) where timers were
reinitialized using InitWithCallback.

I still need to  figure out a good way to find the nsITimer::Init callers, though.

Comment 12

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

It looks like there's only one caller of nsITimer::Init (nsHttpHandler), and
it's fine.