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

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Carsten Book [:Tomcat]]

Attachment: stack

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

Comment 1

[Bob Clary [:bc] (inactive)]

This is on all platforms going back to Beta/25 at least. It was first seen on August 30.

Comment 2

[Steve Workman [:sworkman] (INACTIVE)]

August 30 is before changes I made to imgStatusTracker. Adding Seth.

Comment 3

[Seth Fowler [:seth] [:s2h]]

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.

Comment 4

[Seth Fowler [:seth] [:s2h]]

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

Comment 5

[Seth Fowler [:seth] [:s2h]]

I'm pretty much convinced that the assertion is wrong here. Which is unfortunate. I thought this might be a deeper bug.

Comment 6

[Seth Fowler [:seth] [:s2h]]

Attachment: Remove erroneous assertion in imgStatusTracker.

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.

Comment 7

[Seth Fowler [:seth] [:s2h]]

That was poor wording. Ignore the word "only".

Comment 8

[Seth Fowler [:seth] [:s2h]]

Thanks for the review, Josh.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f396c436c09c

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f396c436c09c

Comment 10

[Seth Fowler [:seth] [:s2h]]

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.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b2efc76034f8