34663 - crash in nsDNSService::Shutdown()

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[John Bandhauer]

While trying to reproduce bug http://bugzilla.mozilla.org/show_bug.cgi?id=34411 
I got a crash with a much different stack then found there.

Using NT debug build from April 5 noonish sources...

steps:
1) start mozilla
2) unplug network
3) choose a profile
4) wat for alert box saying that it can't open the page
5) quit.

It crashes inside the mLookups.Enumerate in nsDNSService::Shutdown().

At the point it crashes the current hashentry pointer is 0xdddddddd and the 
nentries for the hashtable is 0.

I'm wondering if entries are getting removed as you enumerate and bad things 
happen. When going direct to an nspr hastable it expects the enumeration 
function to return a value saying whether to remove the item or not. 
nsHashtable has a wrpper for that too. But if you are doing an indirect 
removal of items i the enumerator it might be freaking. Then again maybe 
something else is wrong?

Anyway, the stack is attached...

Comment 1

[John Bandhauer]

Attachment: the stack

Comment 2

[gordon]

This looks like it could be a duplicate of bug 33821.  I think it is definitely a 
threading problem and that the lookups aren't getting full protection.  I'm 
pretty sure I know how to fix this, but I'm having trouble reproducing it because 
the nsIOService seems to be getting leaked, so Shutdown() isn't getting called on 
my NT and Mac builds.  I'm currently working on figuring out why...

Comment 3

[Warren Harris]

Moving to M17 which is now considered part of beta2.

Comment 4

[gordon]

The crash looks like it is due to the fact we are trying to Remove() a hash table 
entry while Enumerating() it.  Unfortunately, nsSupportsHashTable wraps the 
enumerate callback function to PL_HashTableEnumerateEntries() so we can't use the 
HT_ENUMERATE_REMOVE flag to mark entries that need to be removed while we are 
enumerating the hash.  To efficiently weed out the hash table, we need to go 
around the nsSupportsHashTable api, and call HashTableEnumerateEntries() 
directly, or just give up on using an nsSupportsHashTable in the first place and 
handle the reference counting ourselves.

Comment 5

[leger]

Adding crash keyword.

Comment 6

[Phil Peterson]

[nsbeta2+]

Comment 7

[gordon]

A fix for this has been checked in.  I'm not sure we'll be able to verify it 
until bug 17390 is fixed.

Comment 8

[Tom Everingham]

per Gordon's comment, waiting on 17390

Comment 9

[Tom Everingham]

verif:
Win NT 2000091505