Assertion failure: progress <= progressMax (unexpected progress values), at nsHttpChannel.cpp:5464, with 206 response

RESOLVED FIXED in mozilla33

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dbaron, Assigned: mayhemer)

Tracking

({assertion, crash})

Trunk
mozilla33
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Posted file debugging
I just hit the assertion:

Assertion failure: progress <= progressMax (unexpected progress values), at /home/dbaron/builds/ssd/mozilla-central/mozilla/netwerk/protocol/http/nsHttpChannel.cpp:5464

while loading the page http://en.wikipedia.org/wiki/Special:Search?search=Juan%20Carlos%20of%20Spain&go=Go (I think; it could have been related to another tab I had loaded).

I was running a Linux64 debug build of what is locally 68e47da259c9, i.e., 
https://hg.mozilla.org/mozilla-central/rev/b9e1856deef1
plus the patches in https://hg.mozilla.org/users/dbaron_mozilla.com/patches/rev/a40808b3382a pushed up to the patch round-viewport-units.

There's a bit of info in the attached gdb session.
nsHttp:4,cache2:5 log would be great to have.  This is usually hard to reproduce (very racy) and is definitely something with concurrent writing and reading on a single cache entry so definitely more then one tab for wikipedia open at the same time I presume.

I have some plans to modify or improve this code in bug 979929.
Assignee: nobody → honzab.moz
Decided to introduce a probe to get more data how often this happens.
Hmm.. this actually comes from OnTransportStatus notification, so this is not related to cache.  This is more related to some weirdness in transaction/connection layers.

David, are you using HTTP pipelining or any special setting for the http backend?
I do have HTTP pipelining turned on.
Assignee: honzab.moz → nobody
Posted patch v1Splinter Review
I think I know the cause here.  I did more careful study of the debugging log from David with some manual tests.  

The bug is not in the cache code but is caused by changes in the http channel introduced to make it work with the new cache.  We forget to move mCachedResponseHead to mResponseHead in case of concurrent cache reading.  This will tell the channel a wrong value of Content-Length of the whole entity.

The patch just effectively adds |mResponseHead = Move(mCachedResponseHead);| to the |if (mConcurentCacheAccess)| branch.  But the code is now identical, so I took it out of each branch to be contained only once.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #8444012 - Flags: review?(michal.novotny)
Posted patch v1 -WSplinter Review
ws ignored for maybe better POV
Attachment #8444012 - Flags: review?(michal.novotny) → review+
https://hg.mozilla.org/mozilla-central/rev/513955ec7eb0
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.