Closed Bug 567680 Opened 10 years ago Closed 10 years ago

nsMemoryCacheDevice::BindEntry increments the entry count for doomed entries

Categories

(Core :: Networking: Cache, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

(Keywords: assertion, intermittent-failure)

Attachments

(1 file)

Attached patch Patch (v1)Splinter Review
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.
Attachment #446988 - Flags: review?(dcamp)
Since bug 541739 is a pretty major issue, and this patch is very simple, a quick review would be appreciated.  :-)
Is dcamp still doing reviews at all?
Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

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

Dunno, can you?  ;-)
Attachment #446988 - Flags: review?(bzbarsky)
Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

No good deed goes unpunished, huh?  ;)
Attachment #446988 - Flags: review?(bzbarsky) → review+
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! :-)
Attachment #446988 - Flags: review?(dcamp)
http://hg.mozilla.org/mozilla-central/rev/99062319b7c2

This is tested by the unit test for bug 541739.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Comment on attachment 446988 [details] [diff] [review]
Patch (v1)

This is needed for the unit test of bug 541739.
Attachment #446988 - Flags: approval1.9.2.5?
Summary: nsMemoryCacheDevide::BindEntry increments the entry count for doomed entries → nsMemoryCacheDevice::BindEntry increments the entry count for doomed entries
Attachment #446988 - Flags: approval1.9.2.7?
Attachment #446988 - Flags: approval1.9.2.6-
Attachment #446988 - Flags: approval1.9.2.5?
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.
Duplicate of this bug: 548406
Depends on: 513074
Whiteboard: [orange]
Attachment #446988 - Flags: approval1.9.2.9?
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.