crashes [@ nsDBAccessor::EnumEntry]

VERIFIED FIXED

Status

()

Core
Networking: Cache
P3
critical
VERIFIED FIXED
18 years ago
18 years ago

People

(Reporter: dbaron, Assigned: neeti)

Tracking

({topcrash})

Trunk
x86
Linux
topcrash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rtm++] r=gagan, SR=buster, crash signature)

Attachments

(2 attachments)

(Reporter)

Description

18 years ago
Lots of crashes with the following stack:

nsDBAccessor::EnumEntry
[d:\builds\seamonkey\mozilla\netwerk\cache\filecache\nsDBAccessor.cpp line 311]
nsDBEnumerator::HasMoreElements
[d:\builds\seamonkey\mozilla\netwerk\cache\filecache\nsDBEnumerator.cpp line 66]
nsReplacementPolicy::AddAllRecordsInCache
[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsReplacementPolicy.cpp line 172]
nsReplacementPolicy::LoadAllRecordsInAllCacheDatabases
[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsReplacementPolicy.cpp line 683]
nsReplacementPolicy::DeleteAtleastOneEntry
[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsReplacementPolicy.cpp line 610]
nsReplacementPolicy::CheckForTooManyCacheEntries
[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsReplacementPolicy.cpp line 453]
nsReplacementPolicy::GetCachedNetData
[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsReplacementPolicy.cpp line 560]
nsCacheManager::GetCachedNetData
[d:\builds\seamonkey\mozilla\netwerk\cache\mgr\nsCacheManager.cpp line 316]
...

have shown up in trunk talkback data starting on 10-18.  See
http://www.mozilla.org/projects/seamonkey/reports/ns6analysis.html#nsDBAccessor::EnumEntry
for details.

I don't know whether these are also in branch talkback data.  Therefore,
nominating for rtm to get immediate attention until someone looks to see that
they actually aren't there as well.
(Reporter)

Updated

18 years ago
Keywords: rtm, topcrash

Comment 1

18 years ago
Yes, this crash i definitely in the latest RTM branch builds.  Although there
isn't a lot of entries yet for the latest "Netscape6.00" tagged builds, this is
#4 in todays rankings.  Here are the entries:


    nsDBAccessor::EnumEntry 0d155744
	http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/filecache/nsDBAccessor.cpp line 311
	Build: 2000101810 CrashDate: 2000-10-18 UptimeMinutes: 83  Total: 83
	OS: Windows NT  4.0 build 1381
	URL: http://www.megapixel.net/html/issueindex.html Comment: I clicked on the
review for the Sony MVC-CD1000 digital camera
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=19335843 StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=19335843
nsDBAccessor::EnumEntry a21d3350
	http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/filecache/nsDBAccessor.cpp line 311
	Build: 2000101810 CrashDate: 2000-10-18 UptimeMinutes: 58  Total: 147
	OS: Windows 98  4.10 build 67766222
	URL: http://www.ign.com/ Comment:
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=19345925 StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=19345925
nsDBAccessor::EnumEntry a21d3350
	http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/filecache/nsDBAccessor.cpp line 311
	Build: 2000101810 CrashDate: 2000-10-19 UptimeMinutes: 323  Total: 323
	OS: Windows NT  4.0 build 1381
	URL:
	Comment:
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=19387964 StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=19387964
nsDBAccessor::EnumEntry a189a619
	http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/filecache/nsDBAccessor.cpp line 311
	Build: 2000101810 CrashDate: 2000-10-19 UptimeMinutes: 111  Total: 111
	OS: Windows NT  4.0 build 1381
	URL:
	Comment:
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=19388077 StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=19388077
nsDBAccessor::EnumEntry ed296e20
	http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/filecache/nsDBAccessor.cpp line 311
	Build: 2000101909 CrashDate: 2000-10-19 UptimeMinutes: 26  Total: 26
	OS: Windows 98  4.10 build 67766446
	URL:
	Comment:
	 Detailed : http://climate/reports/incidenttemplate.cfm?bbid=19423987 StackTrace:
http://climate/reports/stackcommentemail.cfm?dynamicBBID=19423987

Updated

18 years ago
Severity: normal → critical
(Assignee)

Comment 2

18 years ago
Looks like mDB is pointing to a freed value.
Status: NEW → ASSIGNED

Comment 3

18 years ago
Is it possible mDB is null? We currently have

  NS_ASSERTION(mDB, "no database") ;

in EnumEntry but we don't actually fail if mDB is null. We would crash at the 
same place the crashes suggest if we are null.

I can prove to myself how we can get in a state where mDB is null, but I can 
come up with a possible scenario.

nsDBAccessor::Shutdown sets mDB to null.  nsNetDiskCache::DBRecovery eventually 
calls the shutdown function. Is it somehow possible that DBRecovery is getting 
called in the middle of the enumeration and therefore mDB gets set to null and 
HasMoreElements crashes. I can't find a scenario where this will happen since 
DBRecovery in HasMoreElements and GetNext is always followed by an error call 
which bails out of the iterator. But if 
nsReplacementPolicy::AssociateCacheEntryWithRecord were to lead to this then 
perhaps it could happen.

Any way, just a thought. I've never seen this in my debug build though 
according to talkback I've once hit this in a release build.
(Assignee)

Comment 4

18 years ago
Scott,

Exactly, I was thinking along the same lines. We could put a null check for mDB 
in all the nsDBAccessor functions. This would help if we were crashing due a 
null mDB, but not in the case where mDB has a corrupt value.

Neeti

Comment 5

18 years ago
I think it's worthwhile to do just in case it is a null ptr. Returning failure
at this point would lead to DBRecovery being called and I think things would
work out afterwards.  My previous comment meant to say that I can't prove that
it's null but that I can imagine a scenario where it could happen.  
(Assignee)

Comment 6

18 years ago
Created attachment 17973 [details] [diff] [review]
attached patch to check for null value for mDB
(Assignee)

Comment 7

18 years ago
I have attached a patch to ckeck for for null mDB, in all nsDBAccessor 
functions. 

Rick, Gagan,
Can you review the fix?

Thanks,
Neeti

Comment 8

18 years ago
This fix will help boost the MTBF and It is one of topcrashes reported by 
talkback.

Comment 9

18 years ago
null-checks never hurt. Except when they hide other problems. For rtm, this
looks ok to me r=gagan
Whiteboard: [rtm need info]

Updated

18 years ago
Whiteboard: [rtm need info] → [rtm need info] r=gagan, NEED SR=
(Assignee)

Comment 10

18 years ago
Rick, Scott,

Can I get a super review for the attached patch?

Thanks,
Neeti

Comment 11

18 years ago
sr=buster.  simple defensive null checks.  The only (very minor) change I would
ask for is to add
  NS_ASSERTION(mDB, "no database")
before the last two null checks in your patch, so we don't mask the error in
these places. 
Whiteboard: [rtm need info] r=gagan, NEED SR= → [rtm need info] r=gagan, SR=buster
(Assignee)

Comment 12

18 years ago
Created attachment 18051 [details] [diff] [review]
added NS_ASSERTION(mDB, "no database")before the last three null checks

Comment 13

18 years ago
Marking it as rtm+ and forwarding it to pdt.
Whiteboard: [rtm need info] r=gagan, SR=buster → [rtm+] r=gagan, SR=buster

Comment 14

18 years ago
rtm++
Whiteboard: [rtm+] r=gagan, SR=buster → [rtm++] r=gagan, SR=buster
(Assignee)

Comment 15

18 years ago
Waiting for tree to open to check this in.
(Assignee)

Comment 16

18 years ago
Checked in a fix on branch and trunk.
(Assignee)

Comment 17

18 years ago
Marking it fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 18

18 years ago
good news. I finally hit the enumentry crash in my debug build and mDB was null.
So this patch should stop the crash.

Comment 19

18 years ago
I also just hit the nsDBAccessor::Put version of this crash and it too was due
to mDB being null so all of the null ptr checking will pay off.

It was pretty easy for me to hit these by running browser buster. On around the
25th page I hit this. 

Comment 20

18 years ago
see 58171 for the reason I think we were hitting this crash.

Comment 21

18 years ago
not showing up in latest crash reports since 11/5

marking verified

Comment 22

18 years ago
verif.
Status: RESOLVED → VERIFIED
Crash Signature: [@ nsDBAccessor::EnumEntry]
You need to log in before you can comment on or make changes to this bug.