Closed
Bug 76372
Opened 23 years ago
Closed 23 years ago
Trunk crash [@ nsCacheEntryHashTable::GetKey] on exit
Categories
(Core :: Networking: Cache, defect)
Tracking
()
VERIFIED
FIXED
mozilla0.9.1
People
(Reporter: jay, Assigned: gordon)
References
Details
(Keywords: crash, topcrash, Whiteboard: r=gagan sr=brendan, a=brendan/chofmann - lets get this in.)
Crash Data
Attachments
(1 file)
1.42 KB,
patch
|
Details | Diff | Splinter Review |
This crash has crept up to the top of the Talkback topcrash list. Most of the user comments mention a crash on exit and all of them are on Win32 platforms. Here is some info from today's Talkback report which covers the last 10 days worth of crashes: nsCacheEntryHashTable::GetKey 105 First BBID :28865978 Last BBID :29206189 Min Runtime :49 Max Runtime :1279451 First Appearance Date : 2001-04-09 Last Appearance Date : 2001-04-17 First BuildID : 2001040906 Last BuildID : 2001041612 Stack Trace: nsCacheEntryHashTable::GetKey [d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheEntry.cpp line 524] ChangeTable [d:\builds\seamonkey\mozilla\xpcom\ds\pldhash.c line 328] PL_DHashTableEnumerate [d:\builds\seamonkey\mozilla\xpcom\ds\pldhash.c line 481] nsCacheEntryHashTable::Finalize [d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheEntry.cpp line 567] PL_DHashTableFinish [d:\builds\seamonkey\mozilla\xpcom\ds\pldhash.c line 227] nsMemoryCacheDevice::~nsMemoryCacheDevice [d:\builds\seamonkey\mozilla\netwerk\cache\src\nsMemoryCacheDevice.cpp] nsMemoryCacheDevice::`scalar deleting destructor' nsCacheService::Shutdown [d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp line 291] nsCacheService::Observe [d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp line 1071] nsObserverService::Notify [d:\builds\seamonkey\mozilla\xpcom\ds\nsObserverService.cpp line 238] NS_ShutdownXPCOM [d:\builds\seamonkey\mozilla\xpcom\build\nsXPComInit.cpp line 449] netscp6.exe + 0x11d1 (0x004011d1) netscp6.exe + 0x2b92 (0x00402b92) KERNEL32.DLL + 0x192a6 (0x77e992a6) Source File : http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheEnt ry.cpp line : 524 Some urls: (29205606) URL: www.nvidia.com (29202959) URL: http://www.micro-solutions.com/ (29199238) URL: www.nhl.com (29196599) URL: http://www.dailyradar.com/downloads/game_file_558.html (29163691) URL: http://blackboard.com/courses/CMIS320 (29080236) URL: www.spiegel.de (29080025) URL: news/foo.com (crash on startup) (29017382) URL: www.salon.com (29013390) URL: http://developer.java.sun.com/servlet/SessionServlet?url=http://developer.java.s un.com/developer/earlyAccess/j2sdk131/ (28924430) URL: http://www.news.com (28919966) URL: http://www.amexmail.com (28897419) URL: http://www.freshmeat.net And a few recent Talkback entries with detailed info: nsCacheEntryHashTable::GetKey 79d3e336 http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheEnt ry.cpp line 524 Build: 2001041612 CrashDate: 2001-04-17 UptimeMinutes: 86 Total: 89 OS: Windows NT 5.0 build 2195 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=29206189 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=29206189 (29206189) Comments: crash on exit. nsCacheEntryHashTable::GetKey 8d819ecc http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheEnt ry.cpp line 524 Build: 2001041510 CrashDate: 2001-04-16 UptimeMinutes: 19 Total: 30 OS: Windows NT 5.0 build 2195 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=29202959 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=29202959 (29202959) URL: http://www.micro-solutions.com/ (29202959) Comments: Crash on mozilla shutdown. nsCacheEntryHashTable::GetKey d835586b http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheEnt ry.cpp line 524 Build: 2001041612 CrashDate: 2001-04-16 UptimeMinutes: 2 Total: 261 OS: Windows NT 5.0 build 2195 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=29201520 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=29201520 (29201520) URL: news/foo.com (crash on startup) nsCacheEntryHashTable::GetKey 24ec32ff http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheEnt ry.cpp line 524 Build: 2001041522 CrashDate: 2001-04-16 UptimeMinutes: 868 Total: 868 OS: Windows 98 4.10 build 67766446 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=29199238 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=29199238 (29199238) URL: www.nhl.com (29199238) Comments: i was closing it because it couldn't render the page correctly. nsCacheEntryHashTable::GetKey d3c404b9 http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheEnt ry.cpp line 524 Build: 2001041606 CrashDate: 2001-04-16 UptimeMinutes: 281 Total: 281 OS: Windows NT 4.0 build 1381 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=29194233 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=29194233 (29194233) Comments: when exiting the browser on windows after working with ftp directory viewer
Reporter | ||
Comment 1•23 years ago
|
||
Adding Trunk and [@ nsCacheEntryHashTable::GetKey] to summary and crash, topcrash keywords.
Comment 2•23 years ago
|
||
I saw many crashes in NKCACHE.DLL while I shutdown mozilla. But this happend only with my opt build not with my debug so I don´t filled a bug about this (have no stack and no Talkback ID).
Comment 3•23 years ago
|
||
<Hixie> Since this has had no traction and it's now past the 0.9 tree closure, this topcrasher should probably be moved on to another milestone. (Top crashers that are targetted at 0.9 are appearing on the "potential 0.9 fixes" list.)
Reporter | ||
Comment 4•23 years ago
|
||
This is the #2 topcrasher with the latest Trunk builds...it would probably be a good idea to get this fixed in 0.9. That's just my opinion.
Reporter | ||
Comment 5•23 years ago
|
||
Talkback data shows this crash happening with Win32 build 2001041709. Here are one of today's entries: nsCacheEntryHashTable::GetKey 79d3e336 http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheEnt ry.cpp line 524 Build: 2001041709 CrashDate: 2001-04-17 UptimeMinutes: 39 Total: 328 OS: Windows NT 5.0 build 2195 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=29255379 StackTrace: http://climate/reports/stackcommentemail.cfm?dynamicBBID=29255379 (29255379) URL: www.nvidia.com
Brendan, Darin, could you [super]review this patch. I haven't be able to reproduce the crash, but this should be nsCacheEntryHashTable a bit more robust.
Status: NEW → ASSIGNED
Comment 8•23 years ago
|
||
I don't see the point of checking initialized in all these places, given that the caller shouldn't be operating on an uninitialized table, but they don't hurt. The trouble is, how do the four initialized tests help the crash shown in the talkback? That crash seems to come from shutdown time, and the stack shown does not flow through any of these defensive null checks. The stack also doesn't match current nsMemoryCacheDevice::~nsMemoryCacheDevice code -- I don't see any PL_DHashTableFinish call there, now. What would the path in current code be? /be
Comment 10•23 years ago
|
||
> newed sr=
Before that, I newed answers to my questions ;-)
/be
Assignee | ||
Comment 11•23 years ago
|
||
Brendan, you are correct the the additional 'initialized' checks wouldn't prevent the crash describe with the stack trace above. There is already an 'initialized' check in ~nsCacheEntryHashTable() which would have prevented a crash, if that was the problem. If the memory in the hashtable entry is garbage (somehow), like you suggested might be the case the other day, then these 'initialized' checks won't help. Your second question regarding the code not matching may just be an oversight. nsMemoryCacheDevice contains a nsCacheEntryHashTable as a member variable, not a pointer, so we don't have an explicit call to destruct the hashtable in nsMemoryCacheDevice; it gets done automatically when the nsMemoryCacheDevice is destructed. I'll leave it up to you to decide whether these changes should go in for 0.9. I'll keep trying to reproduce it. Hopefully we'll get more clues.
Comment 12•23 years ago
|
||
gordon: thanks for setting me straight -- I skimmed the code too quickly. I think these checks can't hurt, but the one in ~nsCacheEntryHashTable must not have been there for the talkback crashes -- or if it was, it didn't help because initialized was non-zero garbage, along with the table. Anyway, having read the code in more detail, and since other classes keep instances of nsCacheEntryHashTables embedded within their own data members, I think it is right and proper for nsCacheEntryHashTable members to defend against being called on an uninitialized table. Did you cover all the methods that need to check? If so, sr=brendan@mozilla.org. /be
Updated•23 years ago
|
Whiteboard: r=gagan need sr= → r=gagan need sr= 0.9 ?
Comment 13•23 years ago
|
||
a=brendan@mozilla.org for inclusion in 0.9. /be
Updated•23 years ago
|
Whiteboard: r=gagan sr=brendan, a=? → r=gagan sr=brendan, a=brendan/chofmann - lets get this in.
Comment 14•23 years ago
|
||
lets get it checked in.
Assignee | ||
Comment 15•23 years ago
|
||
Patch checked in.
Comment 16•23 years ago
|
||
we think we go this (or at least the part we're going to get for 0.9) Pushing TM out a notch to get this off the 0.9 buglist
Target Milestone: mozilla0.9 → mozilla0.9.1
Assignee | ||
Comment 17•23 years ago
|
||
Brendan, it appears as if pldhashtable is resizing itself during/after finalize, after I've deleted the entries. Hyatt and I were looking at this on his machine earlier this afternoon. Maybe he can post a stack trace. It's still not 100% clear whether the bug is in the pldhashtable code or how the cache code calls it.
Comment 18•23 years ago
|
||
*** Bug 77077 has been marked as a duplicate of this bug. ***
Comment 19•23 years ago
|
||
gordon: show me how ChangeTable might be called (it's static, this is easy to analyze) from JS_DHashTableFinish. Are you sure you aren't seeing something else, like the last statement in Finish, which calls ops->freeTable on the table's entry storage? /be
Assignee | ||
Comment 20•23 years ago
|
||
I just posted a patch in bug 76661 that should fix this bug as well. The nsCacheEntryHashTables do not hold owning references to the cache entries, so I no longer use them to delete remaining entries at shutdown. Instead, I use the eviction list, which *should* be considered the owning reference. All deletion of cache entries is performed off of either the doomed list or eviction list, depending on which list it is on.
Assignee | ||
Comment 21•23 years ago
|
||
Patch has been checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 22•23 years ago
|
||
Just had a crash on exit in w2ksp1 build 2001050320 (post patch). Talkback: TB30074786H
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 23•23 years ago
|
||
That stack trace is actually for bug 78479, which has a patch posted for it. Reclosing this bug.
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Comment 24•21 years ago
|
||
VERIFIED/fixed, per last comment.
Status: RESOLVED → VERIFIED
Depends on: 78479
Updated•13 years ago
|
Crash Signature: [@ nsCacheEntryHashTable::GetKey]
You need to log in
before you can comment on or make changes to this bug.
Description
•