567680 - nsMemoryCacheDevice::BindEntry increments the entry count for doomed entries

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

This function doesn't properly make sure that the entry count is only incremented when the cache entry is actually bound, which can trigger this assertion:

<http://mxr.mozilla.org/mozilla-central/source/netwerk/cache/src/nsMemoryCacheDevice.cpp#130>

I found this out because it happened in the test case for bug 541739 (see bug 541739 comment 8).

The fix is trivial.  I've also fixed the whitespace circus in that function, but basically I just moved the entire code for the function inside the |if (!entry->IsDoomed())| block.

Comment 1

[(no longer active)]

Since bug 541739 is a pretty major issue, and this patch is very simple, a quick review would be appreciated.  :-)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Is dcamp still doing reviews at all?

Comment 3

[(no longer active)]

Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

(In reply to comment #2)
> Is dcamp still doing reviews at all?

Dunno, can you?  ;-)

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

No good deed goes unpunished, huh?  ;)

Comment 5

[(no longer active)]

Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

(In reply to comment #4)
> (From update of attachment 446988 [details] [diff] [review])
> No good deed goes unpunished, huh?  ;)

Indeed it doesn't.  Thanks! :-)

Comment 6

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/99062319b7c2

This is tested by the unit test for bug 541739.

Comment 7

[(no longer active)]

Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

This is needed for the unit test of bug 541739.

Comment 8

[LegNeato]

Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

We will not be taking this for 3.6.6. Moving approval request forward.

If you disagree, send me an email.