Created attachment 363360 [details] valgrind warnings I was just doing a valgrind run on the testcase in bug 399994: I started up firefox with attachment 290918 [details] on the command line and then did File -> Print Preview with the mouse. This warning had showed up either while going into print preview or before; what I was looking for in that bug was what happened when leaving print preview. While doing this valgrind run, I got the attached valgrind warning, which I think is pretty self-explanatory. imgLoader::RemoveFromCache calls entry->SetEvicted() on an entry that was freed inside the call three lines above.
Well, maybe it's not completely self-explanatory since one would expect the caller of NotifyExpired to be holding a reference to the object it's passing. I'm not sure if that's actually the case, though.
Created attachment 363386 [details] [diff] [review] Hold on to a reference to the entry when NotifyExpired is called The expiration tracker doesn't hold on to references to imgCacheEntries, so when an image is expiring, it'll actually be deleted if we don't have an explicit ref to it. This patch adds that ref.
This should block, because it's a problem that's been in since the new image cache, and it's made much worse since bug 466586 landed.
Created attachment 363389 [details] [diff] [review] Hold on to a reference to the entry when NotifyExpired is called (fixed) Accidentally mashed bits of two patches together. Here's the corrected patch.
Could this bug explain the following Firefox 3.1b3pre topcrashes? * #15 - nsExpirationTracker<imgCacheEntry, 3>::RemoveObject(imgCacheEntry*) * #50 - imgLoader::RemoveFromCache(imgCacheEntry*)
Maybe #50, but #15 is unlikely. :(
http://hg.mozilla.org/mozilla-central/rev/0015ec27f938 This'll get pushed to 1.9.1 when bug 466586 does.
You were either lucky or unlucky that you called SetEvicted after you dropped all your references. (It was unlucky because if you'd called SetEvicted earlier then you would still have a cache entry and so wouldn't crash; it was lucky because someone else might not realise and add some code that would then crash.)
Because there's a crash on 1.9.1 which _might_ be caused by this, and because it's clearly something that needed fixing regardless of the status of bug 466586 (that bug depends on this bug, not the other way around), I pushed this to 1.9.1. http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0be041c49b4b