Closed
Bug 1290747
Opened 8 years ago
Closed 8 years ago
Assertion failure: mAnimationState, at RasterImage.cpp:1643
Categories
(Core :: Graphics: ImageLib, defect, P1)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla51
People
(Reporter: cbook, Assigned: seth)
References
()
Details
(Keywords: regression, Whiteboard: [top-sitetestrun])
Attachments
(2 files)
126.52 KB,
text/plain
|
Details | |
1.06 KB,
patch
|
eflores
:
review+
ritu
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Found by bughunter during topsite-testruns on http://www.zing.vn http://persianblog.ir/ while loading this sites with a current windows debug tindebox build. Maybe a regression from bug 1289957 Assertion failure: mAnimationState, at c:/builds/moz2_slave/m-cen-w32-d-000000000000000000/build/src/image/RasterImage.cpp:1643 #01: mozilla_dump_image[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0xd4ed87] #02: imgLoader::SupportImageWithMimeType[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0xd6dbf9] #03: XRE_AddStaticComponent[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x20a694] C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll + 0x232046 #05: mozilla::net::LoadInfo::TriggeringPrincipal[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x4ff7b9] #06: mozilla::net::LoadInfo::TriggeringPrincipal[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x489e8c] #07: UnregisterWeakMemoryReporter[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x1d07c1] #08: UnregisterWeakMemoryReporter[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x1d08ea] #09: XRE_TermEmbedding[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x25e2ac9] #10: XRE_GetFileFromPath[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x25d7dbc] #11: XRE_GetFileFromPath[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x25d8190] #12: XRE_GetFileFromPath[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x25dd432] #13: XRE_main[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\xul.dll +0x25dff3b] #14: ???[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\firefox.exe +0x1c3b] #15: ???[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\firefox.exe +0x15f4] #16: ???[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\firefox.exe +0x201e] #17: TargetNtUnmapViewOfSection[C:\cygwin\mozilla\builds\nightly\mozilla\firefox-debug\dist\bin\firefox.exe +0x319e5] #18: BaseThreadInitThunk[C:\Windows\system32\kernel32.dll +0x4ef1c] C:\Windows\SYSTEM32\ntdll.dll + 0x54608 #20: RtlInitializeExceptionChain[C:\Windows\SYSTEM32\ntdll.dll +0x545db]
Reporter | ||
Comment 1•8 years ago
|
||
edwin, since you reviewed bug 1289957 could you take a look if this is related to this change ?
Flags: needinfo?(edwin)
Reporter | ||
Comment 2•8 years ago
|
||
the tested debug build was based btw on https://hg.mozilla.org/mozilla-central/rev/e5859dfe0bcbd40f4e33f4a633f73ea3473a7849 (just for reference)
Assignee | ||
Comment 3•8 years ago
|
||
Looks like you actually want thread 0's stack: Thread 0 (crashed) 0 xul.dll!mozilla::image::RasterImage::NotifyProgress(unsigned int,mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits> const &,mozilla::Maybe<unsigned int> const &,mozilla::image::SurfaceFlags) [RasterImage.cpp:e5859dfe0bcb : 1630 + 0x2b] eip = 0x5aea028d esp = 0x0021f3f8 ebp = 0x0021f428 ebx = 0x00000001 esi = 0x0000066b edi = 0x0021f4a4 eax = 0x5d889aa8 ecx = 0x6bb2705d edx = 0x77976074 efl = 0x00200202 Found by: given as instruction pointer in context 1 xul.dll!mozilla::image::RasterImage::FinalizeDecoder(mozilla::image::Decoder *) [RasterImage.cpp:e5859dfe0bcb : 1696 + 0x4b] eip = 0x5ae9ed87 esp = 0x0021f430 ebp = 0x0021f4b8 Found by: call frame info 2 xul.dll!mozilla::detail::RunnableFunction<<lambda_1760969938a3f65bda425f7b8ef1273d> >::Run() [nsThreadUtils.h:e5859dfe0bcb : 278 + 0x1b] eip = 0x5aebdbf9 esp = 0x0021f4c0 ebp = 0x0021f4c4 Found by: call frame info 3 xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:e5859dfe0bcb : 1068 + 0xe] eip = 0x5a35a694 esp = 0x0021f4cc ebp = 0x0021f538 Found by: call frame info 4 xul.dll!NS_ProcessNextEvent(nsIThread *,bool) [nsThreadUtils.cpp:e5859dfe0bcb : 290 + 0xd] eip = 0x5a382046 esp = 0x0021f540 ebp = 0x0021f54c Found by: call frame info 5 xul.dll!mozilla::net::nsHttpConnectionMgr::Shutdown() [nsHttpConnectionMgr.cpp:e5859dfe0bcb : 191 + 0xd] eip = 0x5a64f7b9 esp = 0x0021f554 ebp = 0x0021f578 Found by: call frame info 6 xul.dll!mozilla::net::nsHttpHandler::Observe(nsISupports *,char const *,char16_t const *) [nsHttpHandler.cpp:e5859dfe0bcb : 2135 + 0x8] eip = 0x5a5d9e8c esp = 0x0021f580 ebp = 0x0021f69c Found by: call frame info 7 xul.dll!nsObserverList::NotifyObservers(nsISupports *,char const *,char16_t const *) [nsObserverList.cpp:e5859dfe0bcb : 112 + 0x1a] eip = 0x5a3207c1 esp = 0x0021f6a4 ebp = 0x0021f6bc Found by: call frame info 8 xul.dll!nsObserverService::NotifyObservers(nsISupports *,char const *,char16_t const *) [nsObserverService.cpp:e5859dfe0bcb : 305 + 0xe] eip = 0x5a3208ea esp = 0x0021f6c4 ebp = 0x0021f6d8 Found by: call frame info 9 xul.dll!nsXREDirProvider::DoShutdown() [nsXREDirProvider.cpp:e5859dfe0bcb : 1216 + 0x1a] eip = 0x5c732ac9 esp = 0x0021f6e0 ebp = 0x0021f70c Found by: call frame info 10 xul.dll!ScopedXPCOMStartup::~ScopedXPCOMStartup() [nsAppRunner.cpp:e5859dfe0bcb : 1335 + 0xb] eip = 0x5c727dbc esp = 0x0021f714 ebp = 0x0021f720 Found by: call frame info 11 xul.dll!mozilla::DefaultDelete<ScopedXPCOMStartup>::operator()(ScopedXPCOMStartup *) [UniquePtr.h:e5859dfe0bcb : 528 + 0xe] eip = 0x5c728190 esp = 0x0021f728 ebp = 0x0021f728 Found by: call frame info 12 xul.dll!XREMain::XRE_main(int,char * * const,nsXREAppData const *) [nsAppRunner.cpp:e5859dfe0bcb : 4411 + 0x12] eip = 0x5c72d432 esp = 0x0021f730 ebp = 0x0021f75c Found by: call frame info 13 xul.dll!XRE_main [nsAppRunner.cpp:e5859dfe0bcb : 4474 + 0x10] eip = 0x5c72ff3b esp = 0x0021f764 ebp = 0x0021f87c Found by: call frame info 14 firefox.exe!do_main [nsBrowserApp.cpp:e5859dfe0bcb : 247 + 0x16] eip = 0x01271c3b esp = 0x0021f884 ebp = 0x0021fa18 Found by: call frame info 15 firefox.exe!NS_internal_main(int,char * *,char * *) [nsBrowserApp.cpp:e5859dfe0bcb : 380 + 0xf] eip = 0x012715f4 esp = 0x0021fa20 ebp = 0x0021fa74 Found by: call frame info 16 firefox.exe!wmain [nsWindowsWMain.cpp:e5859dfe0bcb : 115 + 0xf] eip = 0x0127201e esp = 0x0021fa7c ebp = 0x0021fab4 Found by: call frame info 17 firefox.exe!__scrt_common_main_seh [exe_common.inl : 255 + 0x1d] eip = 0x012a19e5 esp = 0x0021fabc ebp = 0x0021fb00 Found by: call frame info 18 kernel32.dll + 0x4ef1c eip = 0x770def1c esp = 0x0021fb08 ebp = 0x0021fb0c Found by: call frame info 19 ntdll.dll + 0x54608 eip = 0x77994608 esp = 0x0021fb14 ebp = 0x0021fb4c Found by: previous frame's frame pointer 20 ntdll.dll + 0x545db eip = 0x779945db esp = 0x0021fb54 ebp = 0x0021fb64 Found by: previous frame's frame pointer
Assignee | ||
Comment 4•8 years ago
|
||
This is the assertion: https://dxr.mozilla.org/mozilla-central/source/image/RasterImage.cpp?q=RasterImage%3A%3ANotifyProgress&redirect_type=single#1643 The cause is that a decoder allocated a second frame without calling PostIsAnimated(). This is a little surprising given this other assertion here, which didn't fire: https://dxr.mozilla.org/mozilla-central/source/image/Decoder.cpp#276 That *should've* detected the problem as soon as the frame was allocated. The issue, then, is almost certainly that that code *wasn't executed*, because this check failed: https://dxr.mozilla.org/mozilla-central/source/image/Decoder.cpp#274 I have no idea what the point of that check even is. I walked back through the blame to the version of the file at git revision d8cd01802 (hg 7cef16ef5ac49b176d122632bb0d0d09dbdeaf21) and it appears to me that this is an artifact from an ancient version of this code where the only way to tell if we successfully allocated a new frame was to check the frame out. Obviously we can now directly check whether |mCurrentFrame| contains a frame, and the outer |if| in this block of code *does* check that. The first thing to do is rip out this pointless check, which should then give us a stack at the location where the error is actually introduced.
Assignee | ||
Comment 5•8 years ago
|
||
s/check the frame out/check the frame count/, above.
Assignee | ||
Comment 6•8 years ago
|
||
Heh, actually once I was able to reproduce the problem (couldn't get it to happen with e10s on), the actual issue turned out to be much simpler. If we hit an error, in RasterImage::DoError() we reset |mAnimationState| to |Nothing()|. If this happened before the decoder for an animated image finished running, we could wrongly trip this assert. (Or rather, the assert was incorrect for this case.) This patch fixes the assert. That code I pointed out in comment 4 is still super sketchy, though. I'm going to post a followup to fix that.
Attachment #8776413 -
Flags: review?(edwin)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → seth.bugzilla
Flags: needinfo?(edwin)
Assignee | ||
Comment 7•8 years ago
|
||
(In reply to Seth Fowler [:seth] [:s2h] from comment #6) > That code I pointed out in comment 4 is still super sketchy, though. I'm > going > to post a followup to fix that. That's bug 1290759; patch up.
Reporter | ||
Comment 8•8 years ago
|
||
[Tracking Requested - why for this release]: Bughunter topsiterun found
tracking-firefox50:
--- → ?
Attachment #8776413 -
Flags: review?(edwin) → review+
Updated•8 years ago
|
tracking-e10s:
--- → ?
Pushed by seth.bugzilla@blackhail.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/7d45b49f61d5 Fix bad assert in RasterImage::NotifyProgress(). r=edwin
Updated•8 years ago
|
Priority: -- → P1
Reporter | ||
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7d45b49f61d5
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8776413 [details] [diff] [review] Fix bad assert in RasterImage::NotifyProgress(). Approval Request Comment [Feature/regressing bug #]: Bug 1289957 [User impact if declined]: None, but uplifting this will probably help our developers and testers. [Describe test coverage new/current, TreeHerder]: None alas; it's kinda hard to write a test for this right now. That's something I'm looking into. [Risks and why]: Zero. All this does is change a debug-only assert that was asserting the wrong thing. [String/UUID change made/needed]: None.
Attachment #8776413 -
Flags: approval-mozilla-aurora?
Regression, tracked for 50.
status-firefox50:
--- → affected
Comment on attachment 8776413 [details] [diff] [review] Fix bad assert in RasterImage::NotifyProgress(). Low risk, taking it. Aurora50+
Attachment #8776413 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reporter | ||
Comment 14•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/587904a10d9a
Updated•7 years ago
|
Version: unspecified → Trunk
Comment hidden (spam) |
You need to log in
before you can comment on or make changes to this bug.
Description
•