181592 - [FIX]Crash in [@nsCOMArrayEnumerator::~nsCOMArrayEnumerator] shutting down.

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P1)

Description

[[e-mail] (gone - use [e-mail] instead)]

Build ID: 2002-11-21-08, Windows XP.

Summary: Crash in nsCOMArrayEnumerator::~nsCOMArrayEnumerator shutting down the
browser (only Mail and Nav were open).

Steps to Reproduce:

I haven't been able to reproduce yet, but I had WinDVD running (as well as mIRC)
in the background.  Even though it didn't say 'App not responding' in the
taskmanager, it was taking a long time to close down the Mail window and when it
finally closed down mail, once I closed Nav down, I crashed.  (Catch all that? ;-)

Incident ID 14282742
Stack Signature nsCOMArrayEnumerator::~nsCOMArrayEnumerator f8b23811
Product ID MozillaTrunk
Build ID 2002112108
Trigger Time 2002-11-22 22:57:14
Platform Win32
Operating System Windows NT 5.1 build 2600
Module xpcom.dll
URL visited
User Comments I was just shutting down the browser (it was in the background]
Trigger Reason Access violation
Source File Name c:/builds/seamonkey/mozilla/xpcom/ds/nsArrayEnumerator.cpp
Trigger Line No. 135

Stack Trace

nsCOMArrayEnumerator::~nsCOMArrayEnumerator
[c:/builds/seamonkey/mozilla/xpcom/ds/nsArrayEnumerator.cpp, line 135]
nsCOMArrayEnumerator::`scalar deleting destructor'
ObserverListEnumerator::Release
[c:/builds/seamonkey/mozilla/xpcom/ds/nsObserverList.cpp, line 167]
nsCOMPtr_base::~nsCOMPtr_base
[c:/builds/seamonkey/mozilla/xpcom/glue/nsCOMPtr.cpp, line 65]
nsHttpHandler::OnExamineResponse
[c:/builds/seamonkey/mozilla/netwerk/protocol/http/src/nsHttpHandler.cpp, line 635]
nsHttpChannel::ProcessResponse
[c:/builds/seamonkey/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line 606]
nsHttpChannel::OnStartRequest
[c:/builds/seamonkey/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line 2922]
nsOnStartRequestEvent::HandleEvent
[c:/builds/seamonkey/mozilla/netwerk/base/src/nsRequestObserverProxy.cpp, line 162]
PL_HandleEvent [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c, line 645]
PL_ProcessPendingEvents [c:/builds/seamonkey/mozilla/xpcom/threads/plevent.c,
line 578]
nsEventQueueImpl::ProcessPendingEvents
[c:/builds/seamonkey/mozilla/xpcom/threads/nsEventQueue.cpp, line 392]

Comment 1

[Boris Zbarsky [:bzbarsky]]

        NS_IF_RELEASE(mValueArray[mIndex++]);

is bad.... this expands to:

   PR_BEGIN_MACRO
     if (_ptr) {
       NS_LOG_RELEASE_CALL((_ptr), (_ptr)->Release(), __FILE__, __LINE__);
       (_ptr) = 0;
     }
   PR_END_MACRO

So mIndex++ is executed _4_ times during the NS_IF_RELEASE().  Side effects and
macros don't mix, mmkay?  ;)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Something like this may help

The point being that this makes us leak 3/4 the remaining objects if any and
access out-of-bounds memory if the number of remaining objects is not a
multiple of 4.	That's the only way I see for this code to be crashing,
really... 

In any case, we need to make this change no matter what.

Comment 3

[Scott Collins]

Comment on attachment 107220 [details] [diff] [review]
Something like this may help

nice catch, sr=scc

Comment 4

[Alec Flett]

Comment on attachment 107220 [details] [diff] [review]
Something like this may help

r=alecf

Comment 5

[Boris Zbarsky [:bzbarsky]]

fixed

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED with build 2004-06-03-08 on Windows XP.

I've never seen this particular stack come up since...