Trashing memory in nsDiskCacheDevice::evictDiskCacheEntries()

VERIFIED FIXED

Status

()

Core
Networking: Cache
--
blocker
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Simon Fraser, Assigned: Patrick C. Beard)

Tracking

({smoketest})

Trunk
smoketest
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [cache] fix checked in to DISKCACHE2_BRANCH)

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
evictDiskCacheEntries() seems to be running off the end of the sortedRecords 
array (which shows up as a trashed trailer marker in the memory allocators). This 
prevents me from running my build.
(Reporter)

Comment 1

17 years ago
Created attachment 29731 [details]
Stack of badness in delete[], if you care
(Reporter)

Comment 2

17 years ago
sortedRecords is |mCacheMap->EntryCount()| big, |count| gets bigger than this, so 
we trash the array at 

  sortedRecords[count++] = *record;
(Reporter)

Comment 3

17 years ago
This could easily cause smoketest-blocking crashes
Severity: normal → blocker
Keywords: smoketest
(Assignee)

Comment 4

17 years ago
This will happen when the _CACHE_MAP_ file gets corrupted. I'm working on changes 
in the branch that will make this less likely. I obviously need to make the 
current code more robust against this corruption. I'll come up with a minimal 
patch for now.

Updated

17 years ago
Whiteboard: [cache]
I'm seeing this crash frequently on Linux (debug build from yesterday, and
presumably the frequent crashes in my opt build are caused by the same thing).
This is keeping my mean time between failures down around 5-10 minutes, which
isn't exactly usable.
OS: Mac System 8.5 → All
Hardware: Macintosh → All

Comment 6

17 years ago
I was seeing this at launch on Linux 2001-04-05 build until I blew away my cache
directory.
(Assignee)

Comment 7

17 years ago
This crash is fixed in the DISKCACHE2_BRANCH, which is now MUCH more robust 
against corruption/crashes/etc. I've bumped the cache version number, so in 
future, whenever there's an incompatible format change, the cache you have will 
be clobbered. There's also a dirty flag that gets written into the cache map file 
(_CACHE_MAP_) which gets cleared only if the cache is shutdown cleanly. When the 
disk cache starts up again, it will clobber the cache if the dirty flag is set. 
To try this build, pull like so:

cvs co -r DISKCACHE2_BRANCH mozilla/netwerk/cache/src
cvs co -r DISKCACHE2_BRANCH mozilla/netwerk/cache/public

We should be landing this today, if luck is with us.
Whiteboard: [cache] → [cache] fix checked in to DISKCACHE2_BRANCH

Comment 8

17 years ago
How sure are you that the branch isn't going to regress us further? Since this
isn't keeping the builds from being tested by qa (afaik), i'm inclined to
perform blake's carpool (which has been pushed back three days), and then have
you land your branch, if it's ready.

How extensive are the changes on the branch, and is it really ready to go in?

Comment 9

17 years ago
The branch is very small.  We are just treating it as a regular checkin, after 
getting an sr; we don't need a car pool.  We only have a branch so Patrick and I 
can share our work and verify it on multiple platforms before checking it into 
the trunk.  It's our method of team programming.  Maybe we should have spelled 
the branch name in all lower-case, so as not to arouse suspicion. :-)

Comment 10

17 years ago
the sheriff would like this checked in before the tree opens... are you sr'd?

Comment 11

17 years ago
Yes. sr=darin.

Comment 12

17 years ago
Marking FIXED.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 13

17 years ago
verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.