Closed Bug 1018883 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: Networking: HTTP, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: dbaron, Assigned: mayhemer)

Details

(Keywords: assertion, crash)

Attachments

(3 files)

Attached 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
Attached 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)
Attached 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: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.