97620 - Crash unloading -turbo mozilla from tray icon

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect)

Description

[Martin Poirier]

In today (2001083003) and yesterday's builds mozilla crashes when closind it by
choosing exit from the tray icon.

Reproducible : Always

Steps : 

1.Open mozilla with -turbo switch on

2. Close all mozilla windows (you can browse a little before closing)

3. Choose exit from the tray icon to unload mozilla

4. Mozilla unloads, Crash in nkcache.dll

5. Talkback reports : TB34727011Y, TB34726460Y, TB34726233Z

Comment 1

[Chase Tingley]

Confirming based on talkback id.

Comment 2

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

All three talkback reports are identical:

nsDiskCacheBlockFile::DeallocateBlocks
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheBlockFile.cpp, line 231]
nsDiskCacheMap::DeleteStorage
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheMap.cpp, line 772]
nsDiskCacheMap::WriteDiskCacheEntry
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheMap.cpp, line 662]
nsDiskCacheDevice::DeactivateEntry
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheDevice.cpp, line 435]
nsCacheService::DeactivateEntry
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 1279]
nsCacheService::DeactivateAndClearEntry
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 1441]
PL_DHashTableEnumerate [d:\builds\seamonkey\mozilla\xpcom\ds\pldhash.c, line 459]
nsCacheService::ClearActiveEntries
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 1423]
nsCacheService::Shutdown
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 463]
nsCacheProfilePrefObserver::Observe
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 220]
nsObserverService::Notify
[d:\builds\seamonkey\mozilla\xpcom\ds\nsObserverService.cpp, line 238]
NS_ShutdownXPCOM [d:\builds\seamonkey\mozilla\xpcom\build\nsXPComInit.cpp, line 465]
main [d:\builds\seamonkey\mozilla\xpfe\bootstrap\nsAppRunner.cpp, line 1682] 

Comment 3

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

I'm going to give this to ccarlen based on the checkins that went in between
8-28 and 8-29 (I suspect it's fallout from bug 86021), since the comments in
this bug mention -turbo, and since all the talkback reports of this crash are on
Windows.

Three new topcrash signatures appeared in the builds on 8-29:
nsDiskCacheBucket::CountRecords
nsDiskCacheMap::GetFileForDiskCacheRecord
nsDiskCacheBlockFile::DeallocateBlocks

They were the #1, #2, and #3 topcrashes for the builds of 8-29 and 8-30.  I
think it would be very good to get this sorted out for 0.9.4.

Since I could certainly be wrong about which checkin caused this crash, here's
the checkins link for the relevant time period:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=MozillaTinderboxAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2001-08-28+08%3A00&maxdate=2001-08-29+12%3A00&cvsroot=%2Fcvsroot

Comment 4

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

User comments for nsDiskCacheMap::GetFileForDiskCacheRecord:

     (34746286) Comments: closing Mozilla and Outlook Express at essentially the
                same time
     (34736337) URL: usopen.org
     (34718347) URL: equitymaster.com
     (34718347) Comments: while closing the browser

User commenst for nsDiskCacheBucket::CountRecords:

     (34749638) URL: http://jon.livejournal.com/friends/
     (34749638) Comments: Doing a Shift-Reload
     (34746067) Comments: closing out of netscape
User comments for nsDiskCacheBlockFile::DeallocateBlocks:

     (34728183) Comments: BUG 97620
     (34690038) Comments: crash closing mozilla


FWIW, I put these all on the same bug because I think it's too much of a
coincidence for three cache-related crashes to start on the same day with the
limited number of checkins that were going in during the closure.  However, I
admit they could be different, since, looking at the stacks, only one of the
three happens on cache service shutdown.  Maybe I'm wrong that it's related to
ccarlen's checkin, but there's really not much else I can think of.

Sample stacks for the other two crashes (that aren't above):

         nsDiskCacheMap::GetFileForDiskCacheRecord     
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheMap.cpp  line 793]
         nsDiskCacheMap::DeleteStorage 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheMap.cpp  line 761]
         nsDiskCacheMap::DeleteStorage 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheMap.cpp  line 743]
         nsDiskCacheDevice::DeactivateEntry    
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheDevice.cpp  line 432]
         nsCacheService::DeactivateEntry       
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp  line 1279]
         nsCacheService::CloseDescriptor       
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp  line 1240]
         nsCacheEntryDescriptor::Close 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheEntryDescriptor.cpp  line 338]
         nsCacheEntryDescriptor::~nsCacheEntryDescriptor       
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheEntryDescriptor.cpp  line 49]
         nsCacheEntryDescriptor::`scalar deleting destructor'
         nsCacheEntryDescriptor::Release       
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheEntryDescriptor.cpp  line 32]
         nsCOMPtr_base::assign_with_AddRef     
[d:\builds\seamonkey\mozilla\xpcom\base\nsCOMPtr.cpp  line 59]
         nsHttpChannel::OnStopRequest  
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp  line 2204]
         nsHttpChannel::OnStopRequest  
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp  line 2204]
         nsOnStopRequestEvent::HandleEvent     
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsRequestObserverProxy.cpp  line 162]
         PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c 
line 591]


And:

         nsDiskCacheBucket::CountRecords       
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheMap.cpp  line 65]
         GetCacheEntryBinding  
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheBinding.cpp  line 102]
         nsDiskCacheMap::DoomRecord    
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheMap.cpp  line 736]
         nsDiskCacheDevice::DoomEntry  
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheDevice.cpp  line 518]
         nsCacheService::DoomEntry_Locked      
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp  line 993]
         nsCacheEntryDescriptor::Doom  
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheEntryDescriptor.cpp  line 308]
         nsHttpChannel::CloseCacheEntry
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp  line 886]
         nsHttpChannel::OnStopRequest  
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp  line 2204]
         nsOnStopRequestEvent::HandleEvent     
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsRequestObserverProxy.cpp  line 162]
         PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c 
line 591]

Comment 5

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

I went through 5 of the nsDiskCacheMap::GetFileForDiskCacheRecord talkback
reports on http://climate.mcom.com/reports/incidenttemplate.cfm?bbid=NNNNNNNNN ,
and three of them were using -turbo on the command line and two were not.  (Is
there a way for turbo to be invoked without the command line option?)

Anybody else have any suggestions for what might have caused this?

Comment 6

[Conrad Carlen (not reading bugmail)]

  This problem (whatever it is) was exposed (not caused) by bug 86021. That code
causes the cache's profile change observer to be used. This cache's profile
change observer has been around for a while.
  What it may have to with is this: In -turbo mode, closing the last window
causes the cache to partially shut down in response to the profile being shut
down. When the user than exits from the tray icon, the cache will fully shut
down in response to the "xpcom-shutdown" notification. Looking at the code, that
shouldn't be a problem, though I could be missing something. I have not been
able to reproduce this. Still looking.

Comment 7

[Jan Carpenter]

adding "trunk" to summary for tracking

Comment 8

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

Any status on this bug?  Should it go to someone who works on the cache?

Comment 9

[Conrad Carlen (not reading bugmail)]

I haven't been able to reproduce this and have been working on another bug. I'll
be back on this tommorrow.

Gordon, can you take a look at the situation of:
the disk cache device being shut down in response to a profile shutdown
followed immediately by the cache service shutting down in response to
xpcom-shutdown?

It's a but easier to read on the Talkback report itself. Here's one:
http://climate/reports/singleincidentinfo.cfm?dynamicBBID=TB34727011Y

The thing which seems odd to me is that, at this point, if the disk cache device
is shut down, why is nsDiskCacheDevice::DeactivateEntry on the stack?
 

Comment 10

[Peter Trudelle]

shouldn't this be targetted for 0.9.4?

Comment 11

[gordon]

Conrad, what is the sequence of events for turbo mode?  Profile shutdown
followed by xpcom shutdown?

I'll look at this more this afternoon.  Setting target milestone to 0.9.4,
presuming turbo mode will be on by default.  ...but on what platforms?

Comment 12

[Conrad Carlen (not reading bugmail)]

Right - profile shutdown, followed by xpcom shutdown.
It's Windows only.

Comment 13

[Terri Preston]

I was able to reproduce this on the first try and can do it consistantly.

Windows 98 build 2001-09-07-09-trunk 

1. I installed build (with the Quicklaunch option checked) 
2. Surfed a couple of sights 
3. Closed the window (using the x in the upper right corner)
4. Right clicked the try icon and selected exit
5. Crash

Talkback Incident ID 35093143 (I will try on other Windows OS's)

Comment 14

[Conrad Carlen (not reading bugmail)]

Terri, looking at the stack trace, that was a different crash and was very
reproducible. That was a regression from fix to bug 97514 which was in for a
short time yesterday and backed out. See comments on that bug about how it
crashed turbo.

Comment 15

[Martin Poirier]

Update : I'm the reporter of this bug and here is a little update.

I still get the crash on exit from the tray icon (still always reproducible).
However, the problem seems to have changed a little. While I had a crash in
nkcache.dll, mozilla now crashes in appshell.dll. Here is the win98 error report.


MOZILLA a causé une défaillance de page dans
 le module APPSHELL.DLL à 017f:600b6e7e.
Registres :
EAX=00000000 CS=017f EIP=600b6e7e EFLGS=00010206
EBX=00000000 SS=0187 ESP=0068f9a0 EBP=0068f9bc
ECX=02b5de70 DS=0187 ESI=00875340 FS=12bf
EDX=00000002 ES=0187 EDI=00000000 GS=0000
Octets à CS : EIP :
8b 03 8d 4d 0c 51 53 89 7d 0c ff 50 0c 39 7d 0c 
État de la pile :
60f0cc2d 80000000 00000000 0068fa78 004058b2 007b0ed0 00000000 0068fa78 00404a2b
02b5de70 00000000 0068fa9c 00000000 0068fae6 03cf000a 276c0182 


This happened to me with the latest nightlies, while the crash was in nkcache
before. Here are some new Talkback reports with the appshell.dll crash

TB35104261Z
TB35105423M
TB35117132X
TB35118252H
TB35119353x
TB35120618E
TB35127357X

Comment 16

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

Attachment: TB35104261

Comment 17

[Conrad Carlen (not reading bugmail)]

Martin, Stephen - read my comment from 2001-09-08 06:15. 

Comment 18

[Conrad Carlen (not reading bugmail)]

Again, (read the comments) the crash in appshell is not this bug, was a
regression from bug 97514, and has been fixed. Let's keep this bug where it
started - crashes in the cache when unloading -turbo from tray icon.

Comment 19

[Dawn Endico]

For sept 7, 8 and 9, this is the number 3 topcrash bug with 22 crashes. all
on win32. Could someone please please please figure out how to reproduce it.

Comment 20

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

endico was only counting one of the stack signatures.  If you combine all of
them, it's #2 with 50 crashes, and I think #1 was already fixed (smoketest
blocker from Friday).  #3 is bug 99052 with 27, and I don't think there's a
clear #4.  (See my tables at
http://www.people.fas.harvard.edu/~dbaron/mozilla/#talkback .)

Comment 21

[(Don't use, old account) - Kai Engert]

I think I can reproduce this. Do the following: Start mozilla using "mozilla
-turbo". Double click the tray icon to open the browser. Enter the URL of a page
that needs a while to load and has many images, e.g. "www.spiegel.de". While the
page is still loading, quickly click the upper right corner icon to close the
browser. It crashes for me all the time with a stack of nkcache, necko, xpcom.

Comment 22

[Stephen Walker]

I can reproduce the crash using Kai's steps with a recent cvs build on win2k.

Comment 23

[Darin Fisher]

Attachment: v1.0 patch

Comment 24

[Darin Fisher]

gordon and i came up with a fix for this bug... ccarlen: seems to work for us,
can you give it a try?

briefly, on a profile shutdown we enumerate the active cache entries and doom
them.  we then clear the doom list, which detaches any open descriptors from the
cache entries.  this solves the problem without any negative side-effects.

Comment 25

[Conrad Carlen (not reading bugmail)]

Before the patch, using Kai's test, it crashed 3 of 3 times.
After applying the patch, it succeeded 3 of 3 times :-)

Comment 26

[Conrad Carlen (not reading bugmail)]

Gagan, can you r=? Rick, can you sr=?

Comment 27

[Gagan]

Comment on attachment 48972 [details] [diff] [review]
v1.0 patch

in addition to the assertions I'd suggest you also check to see if the entry or the array are valid. So--

if (array && entry) {
   array->AppendElement(entry);
   entry->Mark...
}
This will prevent any random crashes becuz of invalid entry or array.

Comment 28

[Gagan]

Comment on attachment 48972 [details] [diff] [review]
v1.0 patch

since my stuff is mostly nitpicky, I'll go ahead and add the r=. whoever checks this in can decide to take my review changes.

Comment 29

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 48972 [details] [diff] [review]
v1.0 patch

sr=[e-mail]

Comment 30

[Asa Dotzler [:asa]]

Comment on attachment 48972 [details] [diff] [review]
v1.0 patch

a=asa for checkin to 0.9.4 branch.

Comment 31

[rpotts (gone)]

r=[e-mail]

My only comments relate to the code snippet below:

+    nsCacheEntry * entry;
+    for (PRInt32 i=0; i<array.Count(); ++i)
+        DoomEntry_Locked((nsCacheEntry *) array[i]);

it looks like 'entry' is an unused variable.  Could we hoist the array.Count()
out of the for loop... So it is not continually (and needlessly) re-evaluated?

-- rick

Comment 32

[gordon]

I'll incorporate Rick's feedback and submit a new patch.

Comment 33

[Christopher Blizzard (:blizzard)]

Attachment: patch with rick's comments

Comment 34

[Brendan Eich [:brendan]]

Comment on attachment 49112 [details] [diff] [review]
patch with rick's comments

All good.

/be

Comment 35

[Christopher Blizzard (:blizzard)]

Checked into 0.9.4 branch.  I leave it up to gordon to get it into the tip.

Comment 36

[Jay Patel [:jay]]

Looking at Talkback data, there seem to be a few more "nsDiskCache*" stack
signatures showing up in topcrash reports.  dbaron already mentioned the top 3:

nsDiskCacheBucket::CountRecords
nsDiskCacheMap::GetFileForDiskCacheRecord
nsDiskCacheBlockFile::DeallocateBlocks

And further down the topcrash list there are these:

nsDiskCacheBlockFile::AllocateBlocks
nsDiskCacheDevice::OnDataSizeChange

Just wanted to mention them here in case they show up later on.  

Comment 37

[Asa Dotzler [:asa]]

0.9.4 is out the door.

Comment 38

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

Any chance this could get landed on the trunk soon?  It's still among the top
trunk topcrashes.

Comment 39

[gordon]

Fix checked into trunk.

Comment 40

[gordon]

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

Comment 41

[Tom Everingham]

verified:  win98 build 2001111303

Comment 42

[Brant Gurganus]

shortening summary per justdave's orders, summaries will have limit in near future