Closed Bug 325765 Opened 19 years ago Closed 19 years ago

Cache causes crash crossing architecture [@ nsDiskCacheRecord::HashNumber].[@ nsDiskCacheMap::FindRecord]


(Core :: Networking: Cache, defect)

1.8 Branch
Not set





(Reporter: stuart.morgan+bugzilla, Assigned: mark)


(Keywords: fixed1.8.0.2, fixed1.8.1, Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5][camino-1.0.6])


(4 files)

Try saying that three times fast.

Camino and Firefox will both crash on first lanch if the previous launch was under the opposite architecture (because someone copied their entire library from an old machine, got a universal version after having run under Rosetta, or just toggled Rosetta on and off for kicks).  The crash doesn't reproduce if you manually delete the cache before launching the other way.
Oh yeah.  It's all about the cache.

Also TB14724380H.
Component: Profile: BackEnd → Networking: Cache
Attachment #210651 - Attachment description: Stack trace → Stack trace (ppc with x86 cache)
Severity: normal → critical
Summary: Cache causes crash crossing architecture → Cache causes crash crossing architecture [@ nsDiskCacheRecord::HashNumber].[@ nsDiskCacheMap::FindRecord]
This is crashing because the nsDiskCacheHeader::mBucketUsage array is not being byte-swapped when read from/written to disk.  See Swap and Unswap:

nsDiskCacheMap::FindRecord was then getting bad data for the number of buckets on a table entry, so attempts to get the buckets would fail.  Instead of records[1], it would look for records[16777216].  Kaboom.

The cache would be marked dirty on first launch before crashing and therefore discarded on a subsequent launch.

The disk cache is supposed to be endian-neutral and stored on disk in big-endian format.  (Re)introducing proper byte-swapping renders all existing caches unusable on little-endian systems (for the above reason), so the disk cache version number is bumped.  I selected 1.9 for the trunk, reserving 1.8 for the same change on the branch - the trunk and branch are currently at 1.7 and 1.6, respectively.
Attachment #210674 - Flags: review?(darin)
Attached patch Branch versionSplinter Review
QA Contact: profile-manager-backend → networking.cache
Comment on attachment 210674 [details] [diff] [review]
Byte-swap the entire cache header

Attachment #210674 - Flags: review?(darin) → review+
Attachment #210674 - Flags: superreview?(sfraser_bugs)
Attachment #210674 - Flags: superreview?(sfraser_bugs) → superreview+
mEvictionRank was never swapped, and mBucketUsage was introduced by me.
So, at least for the last one, my apologies for this crasher.
mEvictionRank never hurt because it's never used for pointer arithmetic, only in comparisons.  It was wrong, but apparently nobody ever noticed.
Comment on attachment 210675 [details] [diff] [review]
Branch version

a=darin for the 1.8 branch
Attachment #210675 - Flags: branch-1.8.1+
Fixed on trunk, MOZILLA_1_8_BRANCH, and CAMINO_1_0_BRANCH.
Closed: 19 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [camino-1.0]
Comment on attachment 210675 [details] [diff] [review]
Branch version

I'd like this for
Attachment #210675 - Flags: approval1.8.0.2?
It'd be nice if we could avoid discarding the cache for all FF 1.5 users.  Maybe some migration code is the answer, or perhaps we should make this change be XP_MACOSX only on the 1.8.0 branch.  Neither is a great solution, but tossing out 50 MB of cache for users behind a modem isn't very nice either.
Considering that the only caches out there on Macs right now are big-endian, we can handle this with #ifdef XP_MACOSX and not bumping the cache version on the 1.8.0 branch.  The only people who will get shafted this way are those who used an unofficial Firefox 1.5 x86 build.
Attachment #210720 - Flags: review?(darin)
Attachment #210675 - Flags: approval1.8.0.2?
We can spin a new unofficial x86 Firefox build equivalent to with this 1.8.0 patch, so users running the unofficials will crash first using THAT, and not with an official release.

Attachment #210720 - Flags: review?(darin) → review+
Attachment #210720 - Flags: approval1.8.0.2?
Flags: blocking1.8.0.2+
Comment on attachment 210720 [details] [diff] [review]
1.8.0 branch version

approved for 180 branch, a=dveditz for drivers
Attachment #210720 - Flags: approval1.8.0.2? → approval1.8.0.2+
1.8.0 branch version checked in on 1.8.0 branch.
Keywords: fixed1.8.0.2
Whiteboard: [camino-1.0] → [camino-1.0][camino-1.0.1]
Whiteboard: [camino-1.0][camino-1.0.1] → [camino-1.0][camino-1.0.1][camino-1.0.2]
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3]
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4]
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5]
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5][camino-1.0.6]
You need to log in before you can comment on or make changes to this bug.