481553 - possible for imgRequests to think they're in the cache when they aren't

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Joe Drew (not getting mail)]

This is a problem I've found by code inspection, but I've never been able to cause it to happen in running code. Its nature is such that it could cause bug 480352; if it does fix that problem, though, I have no clue how the patch from bug 466586 could have caused it, nor why it'd only show up on Windows (at least on crash-stats).

If an image load comes in on one page (or on one thread) for an image that's already being loaded on a different page (or on a different thread), we correctly don't cache the second load, and let it load all by its lonesome. However, the second imgRequest still thinks it's cached, and will ask the image cache to remove itself when it's cancelled, etc. The problem is that the cache can't easily tell the difference between duplicate imgCacheEntries/imgRequests for the same URL.

I will have to think hard about how to test this. As I mentioned, I've never seen this happen.

Comment 1

[Joe Drew (not getting mail)]

Attachment: fix

Comment 2

[Joe Drew (not getting mail)]

http://hg.mozilla.org/mozilla-central/rev/dc3e1ab79397

Fingers crossed...

Comment 3

[Boris Zbarsky [:bzbarsky]]

This case (same image requested by multiple pages) does in fact get hit.  Write a test for this, perhaps?  It doesn't seem like it should be that hard with a page and an iframe...

Do we need this on 1.9.1?

Comment 4

[Joe Drew (not getting mail)]

I slightly misread the code. This patch only fixes the case of loading from a different thread; in that case, it was possible for the problem described to happen. So, only if we're in the middle of loading on one thread, and then on a different thread, try to load the same image.

I don't know that this is actually possible, though.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Shouldn't be.  All loads happen on the main thread.

Comment 6

[Joe Drew (not getting mail)]

Stuart, why was there all that thread machinery in the image cache to begin with, then?

Comment 7

[Joe Drew (not getting mail)]

Stuart tells me the thread stuff is in imglib for the ability to decode on different threads, which we don't do right now.

Comment 8

[Joe Drew (not getting mail)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f1e2b478fae1