Closed Bug 922613 Opened 7 years ago Closed 7 years ago

Assertion failure: !mHadLastPart || aOther->mHadLastPart (mHadLastPart should be monotonic), at /debug-builds/mozilla-central/mozilla-central/image/src/imgStatusTracker.cpp:629


(Core :: ImageLib, defect)

Not set



Tracking Status
firefox24 --- unaffected
firefox25 --- wontfix
firefox26 --- wontfix
firefox27 + fixed
firefox28 --- fixed
firefox-esr17 --- unaffected
firefox-esr24 --- ?
b2g18 --- unaffected


(Reporter: cbook, Assigned: seth)



(Keywords: assertion, regression, Whiteboard: [qa-])


(2 files)

Attached file stack
found via bughunter while checking 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.
Flags: needinfo?(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.
Flags: needinfo?(seth)
QA Contact: seth
Assignee: nobody → seth
QA Contact: seth
Blocks: 905468
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
> #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
> #17 0x00007fff864721a3 in -[NSApplication run] ()
> #18 0x0000000103685501 in nsAppShell::Run (this=0x10f2e40c0) at
> #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".
Attachment #824892 - Flags: review?(josh) → review+
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+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.