583205 - Crash [@ nsImageToPixbuf::ImageToPixbuf(imgIContainer*)] at the time of download completion

Status: VERIFIED FIXED

(Core :: Widget: Gtk, defect)

Description

[Alice0775 White]

User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b3pre) Gecko/20100729 Firefox/4.0b3pre ID:20100729030150
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b3pre) Gecko/20100729 Firefox/4.0b3pre ID:20100729030150

When I download latest hourly build,
Browser crashes with crash reports at the time of download completion.

bp-4656e4de-1a13-4445-9aec-38b402100730

Reproducible: Always

Steps to Reproduce:
1. Start Minefield with new profile
2. Open URL ( http://hourly-archive.localgho.st/hourly-archive2/mozilla-central-linux/ )
3. Click a link and save to disk

Actual Results:
 Browser crashes with crash reports at the time of download completion.

Expected Results:
 Browser should not crash.

Comment 1

[Alice0775 White]

Regression window:

Works:
http://hg.mozilla.org/mozilla-central/rev/59a9d183d1ec
Mozilla/5.0 (X11; Linux i686; rv:2.0b3pre) Gecko/20100726 Firefox/4.0b3pre ID:20100726134206

Fails:
http://hg.mozilla.org/mozilla-central/rev/09bd4d9ffd66
Mozilla/5.0 (X11; Linux i686; rv:2.0b3pre) Gecko/20100726 Firefox/4.0b3pre ID:20100726135310

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=59a9d183d1ec&tochange=09bd4d9ffd66

Candidate:
Bug 580669 - Mac Menu icon crash with enabled image discarding [@ imgContainer::SyncDecode]

Comment 2

[timeless]

Signature	nsImageToPixbuf::ImageToPixbuf
UUID	4656e4de-1a13-4445-9aec-38b402100730
Time 	2010-07-30 03:05:18.799299
Uptime	73
Last Crash	1456203 seconds (2.4 weeks) before submission
Install Age	225 seconds (3.8 minutes) since version was first installed.
Product	Firefox
Version	4.0b3pre
Build ID	20100729030150
Branch	2.0
OS	Linux
OS Version	0.0.0 Linux 2.6.24-28-generic #1 SMP Fri Jun 18 12:02:15 UTC 2010 i686
CPU	x86
CPU Info	GenuineIntel family 6 model 23 stepping 10
Crash Reason	SIGSEGV
Crash Address	0x18
User Comments	Browser crash when just finishing download latest hourly on latest nightly
Processor Notes 	
EMCheckCompatibility	False
Related Bugs

Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	libxul.so 	nsImageToPixbuf::ImageToPixbuf 	widget/src/gtk2/nsImageToPixbuf.cpp:76
1 	libmozgnome.so 	nsAlertsIconListener::OnStopFrame 	toolkit/system/gnome/nsAlertsIconListener.cpp:204
2 	libxul.so 	imgRequestProxy::OnStopFrame 	modules/libpr0n/src/imgRequestProxy.cpp:576
3 	libxul.so 	imgRequest::OnStopFrame 	modules/libpr0n/src/imgRequest.cpp:624
4 	libxul.so 	nsPNGDecoder::EndImageFrame 	modules/libpr0n/decoders/png/nsPNGDecoder.cpp:236
5 	libxul.so 	nsPNGDecoder::NotifyDone 	modules/libpr0n/decoders/png/nsPNGDecoder.cpp:427
6 		@0xa8e41bff 	
7 	libxul.so 	MOZ_PNG_push_have_end 	modules/libimg/png/pngpread.c:1906
8 	libxul.so 	MOZ_PNG_push_read_chunk 	modules/libimg/png/pngpread.c:364
9 	libxul.so 	MOZ_PNG_process_data 	modules/libimg/png/pngpread.c:41
10 	libxul.so 	nsPNGDecoder::Write 	modules/libpr0n/decoders/png/nsPNGDecoder.cpp:407
11 	libxul.so 	imgContainer::WriteToDecoder 	modules/libpr0n/src/imgContainer.cpp:2232
12 	libxul.so 	imgContainer::AddSourceData 	modules/libpr0n/src/imgContainer.cpp:1251
13 	libxul.so 	imgContainer::WriteToContainer 	modules/libpr0n/src/imgContainer.cpp:2678
14 	libxul.so 	nsPipeInputStream::ReadSegments 	xpcom/io/nsPipe3.cpp:799
15 	libxul.so 	imgRequest::OnDataAvailable 	modules/libpr0n/src/imgRequest.cpp:1052
16 	libxul.so 	ProxyListener::OnDataAvailable 	modules/libpr0n/src/imgLoader.cpp:1989
17 	libxul.so 	nsJARChannel::OnDataAvailable 	modules/libjar/nsJARChannel.cpp:905
18 	libxul.so 	nsInputStreamPump::OnStateTransfer 	netwerk/base/src/nsInputStreamPump.cpp:510
19 	libxul.so 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:400

it looks like frame is null.

Comment 3

[Bobby Holley (:bholley)]

This is my bad - I didn't realize when I landed bug 580669 that ImageToPixbuf was called out of a decode event handler. Should be relatively straightforward to fix. I'll get on it.

Comment 4

[Bobby Holley (:bholley)]

Attachment: patch v1

Added a patch for this.

timeless or alice - could you confirm that this compiles/fixes the problem? I don't have a linux setup handy.

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 461688 [details] [diff] [review]
patch v1

>diff --git a/widget/src/gtk2/nsImageToPixbuf.cpp b/widget/src/gtk2/nsImageToPixbuf.cpp
>--- a/widget/src/gtk2/nsImageToPixbuf.cpp
>+++ b/widget/src/gtk2/nsImageToPixbuf.cpp
>@@ -63,20 +63,33 @@ NS_IMETHODIMP_(GdkPixbuf*)
> nsImageToPixbuf::ConvertImageToPixbuf(imgIContainer* aImage)
> {
>     return ImageToPixbuf(aImage);
> }
> 
> GdkPixbuf*
> nsImageToPixbuf::ImageToPixbuf(imgIContainer* aImage)
> {
>+    nsresult rv;
>     nsRefPtr<gfxImageSurface> frame;
>+    rv = aImage->CopyFrame(imgIContainer::FRAME_CURRENT,
>+                           imgIContainer::FLAG_SYNC_DECODE,
>+                           getter_AddRefs(frame));

I prefer nsresult rv = aImage->CopyFrame.

>+    NS_ABORT_IF_FALSE(frame, "Null Frame - About to Segfault!");

Returning null would be better here, I think.
(If the methods used can fail, then this function should either fail or come up with some fallback.)
nsAlertsIconListener::OnStopFrame, at least, handles null.

Comment 6

[Alice0775 White]

(In reply to comment #4)
> Created attachment 461688 [details] [diff] [review]
> patch v1
> 
> Added a patch for this.
> 
> timeless or alice - could you confirm that this compiles/fixes the problem? I
> don't have a linux setup handy.

I applied the patch to local build. then It works without crash.

Comment 7

[Bill Gianopoulos [:WG9s]]

(In reply to comment #4)
> Created attachment 461688 [details] [diff] [review]
> patch v1
> 
> Added a patch for this.
> 
> timeless or alice - could you confirm that this compiles/fixes the problem? I
> don't have a linux setup handy.

This patch resolves the issue for me as well.

Comment 8

[Bill Gianopoulos [:WG9s]]

Since this is not currently an approved blocker, should approval-2.0 be requested to try go get this landed in time for beta3?

Comment 9

[Frederic Bezies]

Looks like it makes Thunderbird crashes on grabbing mail ? Is bug 583300 duplicate of this one ?

Comment 11

[Ian Neal]

(In reply to comment #8)
> Since this is not currently an approved blocker, should approval-2.0 be
> requested to try go get this landed in time for beta3?

Causes crashes in trunk builds of both Thunderbird and SeaMonkey so would be good to get in before beta3 of FF and alpha3 of SM.

Comment 12

[Bobby Holley (:bholley)]

Attachment: patch v2

Added a patch addressing karl's review concerns.

I'm going to use the "developer judgement" rule and check this in right now with a=topcrash. Drivers are free to smack me if they think that's inappropriate.

Comment 13

[Bobby Holley (:bholley)]

> I'm going to use the "developer judgement" rule and check this in right now
> with a=topcrash. Drivers are free to smack me if they think that's
> inappropriate.

Nevermind - thanks Mossop!

Comment 14

[Bobby Holley (:bholley)]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/ee695e20ee8b

Resolving fixed. Given how close we are to the beta shipping, I'd appreciate anyone affected by this issue downloading the next hourly and making sure things are 100% resolved.

Comment 15

[Wayne Mery (:wsmwk)]

gone from crash-stats after 2010-07-31 for thunderbird

Comment 16

[Daniel Veditz [:dveditz]]

We suddenly have a burst of this in FF3.6.13pre, where we've seen 5 crashes in the past two weeks on all released versions and suddenly 25+ on a nightly. It's possibly all from the same person given the identical OS strings so might not be as bad as it looks, but it looks like this guy isn't surfing the internet anymore.

https://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&date=2010-11-15%2014%3A00%3A00&signature=nsImageToPixbuf%3A%3AImageToPixbuf%28imgIContainer*%29

We didn't take bug 580669 on the 1.9.2 branch so that didn't "cause" it. Did something else do it, or was it just bad luck for this one guy?

Comment 17

[Karl Tomlinson (:karlt)]

There are not many changes on 1.9.2 after 3.6.12 but before 20101103033455.
None of them look related.  I doubt it was a recent Mozilla code change that "caused" this.

Not many of our users are using a kernel built in 2006, so it seems most likely that the 3.6.13pre crashes are all one user.

However, it would be simple to insert a NULL check here.
(I'm not familiar with CopyCurrentFrame so don't know what exactly might cause a failure.)

Comment 18

[Daniel Veditz [:dveditz]]

So far looks like it was just one guy crashing 24 times on a single day, and nothing since. Not a blocker for the branch release, but will keep and eye open for it.