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

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: 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.

Comment 1

[Honza Bambas (:mayhemer)]

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.

Comment 2

[Honza Bambas (:mayhemer)]

Decided to introduce a probe to get more data how often this happens.

Comment 3

[Honza Bambas (:mayhemer)]

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?

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I do have HTTP pipelining turned on.

Comment 5

[Honza Bambas (:mayhemer)]

Attachment: v1

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.

Comment 6

[Honza Bambas (:mayhemer)]

Attachment: v1 -W

ws ignored for maybe better POV

Comment 7

[Honza Bambas (:mayhemer)]

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=6c5c6cbad330

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/513955ec7eb0