441563 - Crash [@imgRequest::NotifyProxyListener(imgRequestProxy*)] at tokyomango.com

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P3)

Description

[dolphinling]

http://www.tokyomango.com crashes on load. If the page is loaded in a background tab, it crashes when the tab is viewed.

Example report: http://crash-stats.mozilla.com/report/index/04e83bab-4199-11dd-b315-001cc45a2c28 

Testcase coming, hopefully.

Comment 1

[dolphinling]

Okay, so apparently this crash isn't quite as reproducible as I thought. I can often, but not always, see it by loading the june 2008 archives in a background tab, then switching to them.

Also, I forgot to mention before, this is seen in both 3.0 on windows and the latest (20080624) nightly on linux

Comment 2

[Dave Garrett]

I tested and got it twice here.  (3.0 on Linux)
bp-4beb8cc0-420a-11dd-81e7-001cc45a2c28
bp-9f2e7a6e-420a-11dd-98d0-001321b13766

Both are fairly useless:
0                   @0xffffe410  	
1    libc-2.7.so    libc-2.7.so@0x2b020 	

I've actually gotten a similar trace before, I think.
Also sketchy to reproduce on my end.

Comment 3

[John]

This happened to me on Facebook with JIT enabled: http://crash-stats.mozilla.com/report/index/e89e5980-82e2-11dd-8444-0013211cbf8a

Don't know if that helps.

Comment 4

[Takeshi Nishimura]

I have reproducible steps.

Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081009 Minefield/3.1b2pre
URL: http://tkm.s31.xrea.com/xul/xulmigemo.shtml
Reproducibility: Happens every time.

Steps to Reproduce:
1. Visit http://piro.sakura.ne.jp/xul/_xulmigemo.html - [A]
2. Open with new background tab - http://tkm.s31.xrea.com/xul/xulmigemo.shtml - [B]
   (It is the first link in "Background" section of [A].)
3. Create another new tab and open http://openlab.ring.gr.jp/skk/skk/dic/SKK-JISYO.L - [C]
   (large ~4MB file)
4. Wait till page loaded.
5. switch to the tab [B] opened in 2.
6. Crash.
7. Clear cache & all tabs, then try again. It will crash again.

Crash reports:
92c676e8-96ae-11dd-bf36-001cc4e2bf68
fc82dc68-967d-11dd-98e9-001a4bd43ef6

I've tested:
3.1b2pre(trunk) - crash
3.0.3 - crash
3.0.3ubuntu - crash
3.0.2 - crash
2.0.0.17ubuntu - OK

Comment 5

[Takeshi Nishimura]

(In reply to comment #4)
Sorry. These are the links to my crash reports:

http://crash-stats.mozilla.com/report/index/92c676e8-96ae-11dd-bf36-001cc4e2bf68
http://crash-stats.mozilla.com/report/index/fc82dc68-967d-11dd-98e9-001a4bd43ef6

Comment 6

[Takeshi Nishimura]

Attachment: Partial broken PNG image

This image causes crash after discarded and restored.
It has correct frame information (1x1) but corrupt data.
(Acrually it is created by modifying the last byte of 1x1 black image, from 0x82 to 0xff.)

Comment 7

[Takeshi Nishimura]

Attachment: Simple crash test page

Simple reproducible steps.
1. Open this link with new background tab.
2. Wait 20 seconds.
3. Switch to the tab created in 1.

This does not cause crash with current trunk (after 2008-10-12) because of flow change by Bug 322475. This is for Firefox 3.0.x branch.

Othre broken images:
http://www.fugitivedesigns.com/tokyomango-v2/images/main_bottom.png
  (linked from http://www.tokyomango.com/)
http://tkm.s31.xrea.com/xul/img/checked.png
  (linked from http://tkm.s31.xrea.com/xul/xulmigemo.shtml)

Comment 8

[Takeshi Nishimura]

Attachment: nsPNGDecoder log

This is caused by Bug 296818. It cannot restore the discarded frame of partially broken image.
At first if the error occurred in decoding, it does not store data to restore later but |mNumFrames| remains 1. If |imgRequest| want to get frame information after discarding, the frame cannot be restored and Firefox will crash. 

Workaround:
Set environment variable MOZ_DISABLE_IMAGE_DISCARD to 1.

Solution:
If the error occurred, it should call |imgContainer::RemoveFrame()| to remove erroneous frames appended after |setjmp()|.
Or simply add broken data by |imgContainer::addRestoreData()|.
Which is preferred?

Comment 9

[Joe Drew (not getting mail)]

Best to add the broken data, so we have the same behaviour before and after discarding.

Taking so it's on my radar, but feel free to create patches :)

Comment 10

[Takeshi Nishimura]

Attachment: Force addRestoreData patch

This patch applies to both 1.9.0 branch and current trunk.

I'm holiday contributer and very old person, so I don't know mochitest or reftest etc., and no time to learn. Feel free to proceed without me.

Comment 11

[Takeshi Nishimura]

Attachment: Crash test case for both trunk and 1.9.0 branch ATTENTION: THIS PAGE WILL CRASH YOUR BROWSER!

This may be my last contribution for this bug.
Simple crash test case which applies to both trunk and 1.9.0 branch.

Steps to crash:
1. Enable JavaScript and open this link with current tab.
2. Wait patiently - about 20 seconds.
3. Crash.

Comment 12

[Daniel Veditz [:dveditz]]

Too late for 1.9.0.4, moving request to next release.

Vlad or Joe: please review this crash patch.

Comment 13

[Joe Drew (not getting mail)]

Attachment: slightly different take on the patch

This patch builds on the previous one, but instead of duplicating the AddDiscardData call, I just move it above the original test. I'm honestly unsure whether this will cause a problem -- Stuart, I'd love to hear your thoughts.

If it doesn't cause any problems, this is how I'd prefer the bug to be fixed.

Comment 14

[Joe Drew (not getting mail)]

Attachment: Interdiff

This is an interdiff between the old patch and the new one.

Comment 15

[dolphinling]

This currently (due to a socorro bug?) holds 70 of the top 100 topcrash spots for 3.1b1.

Comment 16

[Samuel Sidler (old account; do not CC)]

It's actually #1 for Firefox 3.1b1 (please use queries for topcrash reports) but isn't in the top 100 for 3.0.3.

Comment 17

[Samuel Sidler (old account; do not CC)]

Not blocking 1.9.0.5, but we'll *definitely* look at a reviewed and baked patch for approval. Please request approval1.9.0.5 when such a patch is ready.

Comment 18

[Samuel Sidler (old account; do not CC)]

Stuart, can we get your review here? I'd really like to see this in Firefox 3.1b2 since it's the #1 crash in beta 1.

Comment 19

[Joe Drew (not getting mail)]

Checked in to Firefox 3.1 - http://hg.mozilla.org/mozilla-central/rev/6f6c3001e6ff

Leaving this open until it can be backported to 1.9.0.

Comment 20

[Samuel Sidler (old account; do not CC)]

(In reply to comment #19)
> Leaving this open until it can be backported to 1.9.0.

Drivers prefer bugs that have landed on the trunk to be resolved as fixed and use flags to track branch work instead.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Is there any reliable testcase? None of the referenced links or the attached testcase crashes todays Minefield on Vista.

Comment 22

[John]

Are we sure this is absolutely fixed? I just got this crash on today's build again.

Steps to reproduce:
1) Go to http://www.gasbuddy.com/GB_Map_Gas_Prices.aspx
2) Scroll around on the interactive map for about 2 minutes (at this point, the browser leaks massive amounts of memory. For me, it was using 600MB and I only have 1GB, and it crashes soon after. could be memory corruption.

JIT Chrome is enabled (I don't know if that affect it, but the browswer UI lags right before it crashes)

Crash Report: http://crash-stats.mozilla.com/report/index/6092cbf3-ae98-11dd-9c7f-001cc45a2c28

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, I was able to reproduce this crash with a 3.0.5pre build. I'll try to create a testcase for it so we could have a crashtest for it.

John, it looks like your issue is differnet from this one. Please file a new bug and leave the bug # here. Thanks.

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Joe, any way to get a testcase without the need of loading a page in a background tab first? Wouldn't a crashtest suite for libpr0n be helpful? As I can see there is none yet in the repository.

Further there exist a lot of crashes with the same stack as reported in comment 0 with the latest builds.

bp-60633163-ae98-11dd-9eb2-001cc4e2bf68
bp-d80647e6-ae9c-11dd-9746-001a4bd43e5c
bp-60633163-ae98-11dd-9eb2-001cc4e2bf68

Doesn't look like it's fixed => reopening.

Comment 26

[Joe Drew (not getting mail)]

I can't reproduce this bug on any of the URLs in this bug, and breakpad doesn't surface the URLs. Does anyone have any steps-to-reproduce on current trunk?

Comment 27

[John]

You can't reproduce it from Comment 22? It's not just a pageload that crashes the browser. You need to scroll around for a good 2 minutes on the map.

Comment 28

[Samuel Sidler (old account; do not CC)]

My gut feeling is that just because the top of the stack is the same, comment 22 and on is a different crash and should be a different bug.

Comment 29

[John]

Then I'll reopen bug 463926 (Comment 22 and on). Sorry for the confusion.

Comment 30

[Joe Drew (not getting mail)]

Please open a new bug for the crashes (once you've got steps to reproduce) and assign it to me. I'm calling this one fixed.

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+1]]

No more crash reports for b2 and b3pre on all platforms/OS. I think we can safely mark this as verified.

Do we have crashtests for imagelib we could add attachment 343921 [details] to?

Comment 32

[Boris Zbarsky [:bzbarsky]]

Not yet, but feel free to add this as the first one?