156581 - offline: exiting app while offline crashes

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Backend, defect, P1)

Description

[grylchan]

Spun off bug 156187.

Using commercial trunk
2002070808 on linux 2.2 TB8146931
2002070208 on mac 9.2.2 TB8154605

If you are in classic theme (though did see the crash in modern
but always crashes in classic), in messenger, offline, if
you view a mesg source of downloaded mesg, then go to browser
or messenger and do file|quit it will cause a crash.

stack trace
libc.so.6 + 0x7b38e (0x4052838e) 
libc.so.6 + 0x7b334 (0x40528334) 
PL_strfree() 
nsSocketTransportService::ClearEntry() 
PL_DHashTableFinish() 
nsSocketTransportService::Shutdown() 
nsIOService::SetOffline() 
nsIOService::Observe() 
nsObserverService::NotifyObservers() 
NS_ShutdownXPCOM() 
netscape-bin + 0x97ec (0x080517ec) 
libc.so.6 + 0x1bb65 (0x404c8b65) 

Steps to reproduce:
-start Mozilla
-start MailNews
-going offline
-view message (the original message with that the crash occured for the first
time)
-view message source (NO crash)
-quit
=>Crash

I was viewing source of text mesg and then went to Messenger or Browser
and did file|quit.

I tried various window trunks and crashe logs weren't same as linux/mac
first windows: TB8156300
ntdll.dll + 0x4a776 (0x77fca776) 
ntdll.dll + 0x4a635 (0x77fca635) 
MSVCRT.DLL + 0x113d (0x7800113d) 
MSVCRT.DLL + 0x1f5f (0x78001f5f) 

2nd windows TB8154910X:

ntdll.dll + 0x215c (0x77f5215c) 
ntdll.dll + 0x1fbf (0x77f51fbf) 
msvcrt.dll + 0x1ab2e (0x77c2ab2e) 
AtomImpl::`scalar deleting destructor' 
AtomTableClearEntry [c:/builds/seamonkey/mozilla/xpcom/ds/nsAtomTable.cpp, line 
108] 
PL_DHashTableFinish [c:/builds/seamonkey/mozilla/xpcom/ds/pldhash.c, line 324] 
NS_PurgeAtomTable [c:/builds/seamonkey/mozilla/xpcom/ds/nsAtomTable.cpp, line 
156] 
NS_ShutdownXPCOM [c:/builds/seamonkey/mozilla/xpcom/build/nsXPComInit.cpp, line 
693] 
main [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1816] 
WinMain [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1826] 
WinMainCRTStartup() 
kernel32.dll + 0x1eb69 (0x77e7eb69)

Comment 1

[David :Bienvenu]

actually, this is much simpler. Start up, go offline, and exit the app, and
you'll see the same crash. My guess is that the necko shutdown logic/cleanup has
changed recently - cc'ing darin. One clue is that in
nsSocketTransportService::Shutdown(void), I'm hitting the following assertion:
  } else {
    NS_NOTREACHED("Should have thread when shutting down.");
    rv = NS_ERROR_FAILURE;
  }
I wonder if we're calling shutdown twice, or too late? I'll look into it.

Comment 2

[David :Bienvenu]

the checkin for bug 149943 caused this problem. PL_DHashtables can't be
"finished" twice, since they leave all their pointers dangling, as the comment
in the pldhash header file says. I can't see bug 149943, but I'm worried that
the fix in there will be or has been checked into the branch, which would be
very bad. I'll leave it up to Darin as to how he wants to fix this - one fix is
to simply not clear the hash table if we have no thread. The other is to add a
boolean to the sockettransport service which says whether it has been shutdown.
You could also clear out the mHostDB after finishing it (the pldhash.h comments
suggest memsetting it to 0 - I can try that and see if that fixes the crash)

Comment 3

[David :Bienvenu]

Not sure who's filling in for Darin - cc'ing Dougt. It turns out that memset is
not sufficient. It also turns out that, at least for now, this has not been
checked into the branch but afaik, it could be and I can't add comments to that
bug. We could check for a 0 entry count in the hash table before finishing it,
or as I said before, keep track of whether the the host db had been cleared.

Comment 4

[scottputterman]

I've updated the other bug with info about this bug.

Comment 5

[Daniel Veditz [:dveditz]]

Bug 149943 needs to be checked into the branch (soon!) so we need to fix this
crash without backing out 149943.

Comment 6

[scottputterman]

Mitch or Dan,

Any ideas on how we can fix this so we can get 149943 in the branch?

Comment 7

[Paul Wyskoczka]

adding nsbeta1+ and adt1 rtm because it blocks bug 149943

Comment 8

[David :Bienvenu]

Attachment: proposed fix

this just moves the finishing of the hash table inside the if (mThread) code -
if we've already shutdown, we won't try to finish the hash table twice. This
has r=dougt.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 91038 [details] [diff] [review]
proposed fix

sr=dveditz

It would be nice to figure out why we're shutting down twice, but don't hold up
this bug on that account.

Comment 10

[David :Bienvenu]

Dan, that's no mystery - when we go offline, we shutdown the socket transport
service. And when we exit the app, we also shutdown the socket transport service.

Comment 11

[chris hofmann]

Comment on attachment 91038 [details] [diff] [review]
proposed fix

a=chofmann for the trunk 1.1a

Comment 12

[Tuukka Tolvanen (sp3000)]

*sigh*  I already made the same exact patch (in bug 156255) on Monday, I guess
cc'ing darin wasn't the most efficient way of driving that in since he seems to
have wandered off somewhere now for a bit :)

Comment 13

[Daniel Veditz [:dveditz]]

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

Comment 14

[David :Bienvenu]

fixed on trunk on night of 07/11. Please let me know when I can check it in on
the branch.

Comment 15

[scottputterman]

Gary, can you verify this on the trunk?

Comment 16

[grylchan]

sorry lagging...

using commercial trunk

2002-07-12-08-trunk win 2k, mac 9.2.2, mac 10.1.5
2002-07-12-11-trunk linux 2.2

tried quiting while offline in various ways (menu, contrl-q,
clicking the upper right hand x ) and couldn't reproduce
a crash. Tried the original steps to reproduce and it works.
Tested while connected to network/not connected to network.
Tried work offline in profile mgr also.

Tried quitting from different windows (browser, messenger, composer,
addressbook) while offline and no problems.

Tested both modern/classic.

Tried very very fast/brief sanity on offline in general and no problems.

Changing status to verified (on trunk only).

Comment 17

[scottputterman]

adding adt1.0.1+.  Please get drivers approval before checking into the branch.
 Mitch, will you be able to land this one with the fix for 149943?

Comment 18

[Mitchell Stoltz (not reading bugmail)]

Will do.

Comment 19

[chris hofmann]

a=chofmann for 1.0.1  add the fixed1.0.1 keyword after checking into the branch.

Comment 20

[scottputterman]

marked fixed1.0.1 based on comments from Mitch.

Comment 21

[Mitchell Stoltz (not reading bugmail)]

Yes, this has been fixed on the 1.0.1 branch.

Comment 22

[grylchan]

Using commercial branch 
 2002-07-19-08-1.0/ Nt 4.0
 2002-07-19-05-1.0/ mac 9.2.2, mac 10.1.5
 2002-07-18-07-1.0/ linux 2.2

tried quiting while offline in various ways (menu, contrl-q,
clicking the upper right hand x ) and couldn't reproduce
a crash. Tried the original steps to reproduce and 
no crash. Tried w/no network cable connected to computer and no crash.

Tried w/turbo on/off and no crash. Tried starting offline via profile
mgr and no crash. Tried both classic/modern.

verified on branch. 
Changing kw fixed1.0.1 to verified1.0.1