Closed Bug 922613 Opened 7 years ago Closed 7 years ago
Assertion failure: !m
Had Last Part || a Other->m Had Last Part (m Had Last Part should be monotonic), at /debug-builds/mozilla-central/mozilla-central/image/src/img Status Tracker .cpp:629
found via bughunter while checking http://webcams.ukrtelecom.ua/cams?id=40 on a Mac 10.8 Debug Build and crashed with (filed as security bug just in case, feel free to unmark) Assertion failure: !mHadLastPart || aOther->mHadLastPart (mHadLastPart should be monotonic), at /debug-builds/mozilla-central/mozilla-central/image/src/imgStatusTracker.cpp:629 will try to generate testcase etc
This is on all platforms going back to Beta/25 at least. It was first seen on August 30.
OS: Mac OS X → All
August 30 is before changes I made to imgStatusTracker. Adding Seth.
Thanks, this is an extremely useful bug report. This is a result of the patch in bug 905468. It has been causing another problem (bug 910533) which I have found very difficult to debug because I can't reproduce it locally. I'm hoping this is the same issue. Note that I am skeptical that this is a security issue. This assertion just checks that imgStatusTracker's state changes in the way we expect during decoding, but I don't see any obvious way to exploit this.
QA Contact: seth
OK, I'm now actively working on this bug. I successfully reproduced the crash. Here's the stack trace: > #0 imgStatusTracker::Difference (this=0x111615b00, aOther=0x114b5f510) at imgStatusTracker.cpp:627 > #1 0x0000000101c5fccb in mozilla::image::RasterImage::FinishedSomeDecoding (this=0x10ee99300, aIntent=mozilla::image::RasterImage::eShutdownIntent_Done, aRequest=0x122aa7b50) at RasterImage.cpp:3007 > #2 0x0000000101c657ac in mozilla::image::RasterImage::DecodeDoneWorker::Run (this=0x110a11190) at RasterImage.cpp:3462 > #3 0x00000001044520d3 in nsThread::ProcessNextEvent (this=0x100624760, mayWait=false, result=0x7fff5fbfd083) at nsThread.cpp:622 > #4 0x00000001043b2e9c in NS_ProcessPendingEvents (thread=0x100624760, timeout=20) at nsThreadUtils.cpp:201 > #5 0x00000001037374bf in nsBaseAppShell::NativeEventCallback (this=0x10f2e40c0) at nsBaseAppShell.cpp:95 > #6 0x0000000103684a2c in nsAppShell::ProcessGeckoEvents (aInfo=0x10f2e40c0) at nsAppShell.mm:388 > #7 0x00007fff8da25b31 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ () > #8 0x00007fff8da25455 in __CFRunLoopDoSources0 () > #9 0x00007fff8da487f5 in __CFRunLoopRun () > #10 0x00007fff8da480e2 in CFRunLoopRunSpecific () > #11 0x00007fff823ffeb4 in RunCurrentEventLoopInMode () > #12 0x00007fff823ffb94 in ReceiveNextEventCommon () > #13 0x00007fff823ffae3 in BlockUntilNextEventMatchingListInMode () > #14 0x00007fff8647b533 in _DPSNextEvent () > #15 0x00007fff8647adf2 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () > #16 0x0000000103683187 in -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (self=0x10eefc430, _cmd=0x7fff86ca9404, mask=18446744073709551615, expiration=0x422d63c37f00000d, mode=0x7fff7303b1c0, flag=1 '\001') at nsAppShell.mm:165 > #17 0x00007fff864721a3 in -[NSApplication run] () > #18 0x0000000103685501 in nsAppShell::Run (this=0x10f2e40c0) at nsAppShell.mm:742 > #19 0x00000001032addac in nsAppStartup::Run (this=0x10eef4c40) at nsAppStartup.cpp:268 > #20 0x00000001015797ee in XREMain::XRE_mainRun (this=0x7fff5fbfef90) at nsAppRunner.cpp:3976 > #21 0x0000000101579ff9 in XREMain::XRE_main (this=0x7fff5fbfef90, argc=3, argv=0x7fff5fbff890, aAppData=0x7fff5fbff228) at nsAppRunner.cpp:4044 > #22 0x000000010157a45d in XRE_main (argc=3, argv=0x7fff5fbff890, aAppData=0x7fff5fbff228, aFlags=0) at nsAppRunner.cpp:4246 > #23 0x0000000100002297 in do_main (argc=3, argv=0x7fff5fbff890, xreDirectory=0x100631500) at nsBrowserApp.cpp:275 > #24 0x00000001000017d1 in main (argc=3, argv=0x7fff5fbff890) at nsBrowserApp.cpp:635
I'm pretty much convinced that the assertion is wrong here. Which is unfortunate. I thought this might be a deeper bug.
This patch just removes the bad assertion from imgStatusTracker::Difference. It's fairly obvious in retrospect that we may get mHadLastPart set only after starting a decode job. These assertions were speculative anyway.
Attachment #824892 - Flags: review?(josh)
That was poor wording. Ignore the word "only".
Thanks for the review, Josh. https://hg.mozilla.org/integration/mozilla-inbound/rev/f396c436c09c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Comment on attachment 824892 [details] [diff] [review] Remove erroneous assertion in imgStatusTracker. [Approval Request Comment] Bug caused by (feature/regressing bug #): 905468 User impact if declined: 'Crashes' in debug builds. No impact on release builds. Testing completed (on m-c, etc.): On m-c for a few days. Risk to taking this patch (and alternatives if risky): Zero. This patch just removes a wrong assertion. String or IDL/UUID changes made by this patch: None.
Attachment #824892 - Flags: approval-mozilla-aurora?
Attachment #824892 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.