Closed Bug 455606 Opened 16 years ago Closed 16 years ago

hang/asserts after watching webcam


(Core :: Graphics: ImageLib, defect, P2)






(Reporter: joe, Assigned: joe)




(Keywords: fixed1.9.1, regression)


(1 file, 1 obsolete file)

This is fallout from bug 430061, but other things might be related.

If you watch a webcam (such as the one linked) for a short amount of time, the image cache slowly fills up (for a currently unknown reason). Also, we seemingly get lines like the ones below, one for each frame:

++DOMWINDOW == 62 (0x1138e0fc) [serial = 65] [outer = 0x155f5d80]
++DOMWINDOW == 63 (0x1138b18c) [serial = 66] [outer = 0x155f5d80]
++DOMWINDOW == 64 (0x11393b2c) [serial = 67] [outer = 0x155f5d80]

The real problem is that if you then proceed to try to load a new image, you get 

###!!! ASSERTION: imgLoader::CheckCacheLimits -- NULL entry pointer: 'entry', file /Users/joe/mozilla-central/modules/libpr0n/src/imgLoader.cpp, line 687
Why does this block bug 373701?
Blocks: 430061
Flags: blocking1.9.1?
Keywords: regression
Because I can't review it until this is fixed.
Why not, if I might ask?  If it's a matter of not being able to test, that patch should also apply to Firefox 3 branch (and needs to land on there, preferably before the next dot release).
Yeah, it's strictly a testing issue. I didn't think of 1.9.0 - I will try there.
For reasons I don't understand, when you load a webcam like the one linked above, imagelib gets many, many calls to LoadImageWithChannel. Due to a bug in imagelib, we didn't remove the old entries in the cache when we got a new element that couldn't be verified over the network. This patch fixes that by removing pre-existing elements if they're already in the cache before putting a new element into the cache.
Attachment #339300 - Flags: review?(pavlov)
The previous patch had a small nit - we called RemoveFromCache(entry) when |entry| is the cache entry we're trying to add. It didn't actually matter, because of the way RemoveFromCache is implemented, but we shouldn't do that.
Attachment #339300 - Attachment is obsolete: true
Attachment #339301 - Flags: review?(pavlov)
Attachment #339300 - Flags: review?(pavlov)
> imagelib gets many, many calls to LoadImageWithChannel.

This is a full-page image (going through nsImageDocument), which means that you get a LoadImageWithChannel for each channel.  The multipart converter creates one channel per part, so you're getting a LoadImageWithChannel per frame, basically.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Attachment #339301 - Flags: review?(pavlov) → review+
Pushed with
Closed: 16 years ago
Resolution: --- → FIXED
Couple of comments I should have made earlier:

What was happening was that |imgCacheEntry|s were allocated, but PutIntoCache would return false (which wasn't checked) because an entry with that key already existed in the cache. So the caller would continue on, assuming that the element was in the cache.Further, when the request received callbacks as data was received, it'd increase the size of the cache.

Eventually, the base size of the cache would be exceeded with these "phantom entries," leaving the cache-clearing code in an infinite loop as it tried to free elements that didn't exist to reduce the cache below its maximum size.
You need to log in before you can comment on or make changes to this bug.