805355 - startup crash in _pixman_implementation_fill

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Scoobidiver (away)]

It's similar to bug 795234.
It first appeared in 19.0a1/20121024 and is currently #1 top crasher in this build (hit by 10 users). The regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=48502b61a63e&tochange=93cc1ee94291

Signature 	libxul.so@0xca9378 | arm_neon_fill More Reports Search
UUID	1d299365-0800-4ea4-8cef-beca72121024
Date Processed	2012-10-24 21:06:55
Uptime	27
Install Age	16.9 minutes since version was first installed.
Install Time	2012-10-24 20:51:00
Product	FennecAndroid
Version	19.0a1
Build ID	20121024030643
Release Channel	nightly
OS	Android
OS Version	0.0.0 Linux 2.6.29-omap1 #1 PREEMPT Tue Feb 28 23:20:29 CET 2012 armv7l archos/g8/G8A/:2.2.1/FROYO/eng..20120228.232428:user/test-keys
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x52e00000
App Notes 	
AdapterDescription: 'Imagination Technologies -- PowerVR SGX 530 -- OpenGL ES 2.0 -- Model: A70S, Product: Archos 70 Internet Tablet, Manufacturer: archos, Hardware: archos'
EGL? EGL+ GL Context? GL Context+ GL Layers? GL Layers+ 
archos A70S
archos/g8/G8A/:2.2.1/FROYO/eng..20120228.232428:user/test-keys
EMCheckCompatibility	True
Adapter Vendor ID	Imagination Technologies
Adapter Device ID	PowerVR SGX 530
Device	archos A70S
Android API Version	8 (REL)
Android CPU ABI	armeabi-v7a

Frame 	Module 	Signature 	Source
0 	libxul.so 	libxul.so@0xca9378 	
1 	libxul.so 	arm_neon_fill 	pixman-arm-neon.c:226
2 	libxul.so 	_pixman_implementation_fill 	pixman-implementation.c:182
3 	libxul.so 	delegate_fill 	pixman-implementation.c:62
4 	libxul.so 	_pixman_implementation_fill 	pixman-implementation.c:182
5 	libxul.so 	_moz_pixman_fill 	pixman.c:772
6 	libxul.so 	_clip_and_composite_boxes 	cairo-image-surface.c:2938
7 	libxul.so 	_cairo_image_surface_fill 	cairo-image-surface.c:3750
8 	libxul.so 	_cairo_surface_fill 	cairo-surface.c:2348
9 	libxul.so 	_cairo_gstate_fill 	cairo-gstate.c:1290
10 	libxul.so 	_moz_cairo_fill_preserve 	cairo.c:2459
11 	libxul.so 	gfxContext::Fill 	gfxContext.cpp:310
12 	libxul.so 	PresShell::RenderDocument 	nsPresShell.cpp:4304
13 	libxul.so 	mozilla::AndroidBridge::TakeScreenshot 	AndroidBridge.cpp:2501
14 	libxul.so 	ScreenshotRunnable::Run 	nsAppShell.cpp:89
15 	libxul.so 	RunnableMethod<ScreenshotRunnable, tag_nsresult 	tuple.h:383
16 	libxul.so 	MessageLoop::RunTask 	message_loop.cc:333
17 	libxul.so 	MessageLoop::ProcessNextDelayedNonNestableTask 	message_loop.cc:231
18 	libxul.so 	MessageLoop::DoIdleWork 	message_loop.cc:472
19 	libxul.so 	mozilla::ipc::MessagePump::Run 	MessagePump.cpp:109
20 	libxul.so 	MessageLoop::RunInternal 	message_loop.cc:215
21 	libxul.so 	MessageLoop::Run 	message_loop.cc:208
22 	libxul.so 	nsBaseAppShell::Run 	nsBaseAppShell.cpp:163
23 	libxul.so 	nsAppStartup::Run 	nsAppStartup.cpp:290
24 	libxul.so 	XREMain::XRE_mainRun 	nsAppRunner.cpp:3799
25 	libxul.so 	XREMain::XRE_main 	nsAppRunner.cpp:3866
26 	libxul.so 	XRE_main 	nsAppRunner.cpp:3941 

More reports at:
https://crash-stats.mozilla.com/query/query?product=FennecAndroid&query_search=signature&query_type=contains&query=arm_neon_fill&do_query=1

Comment 1

[Scoobidiver (away)]

It stopped in 19.0a1/20121025. The working range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=93cc1ee94291&tochange=5c82f5a5e90d
It might have been caused by the bustage of bug 803013 or bug 795259.

Comment 2

[Scoobidiver (away)]

It's back in 19.0a1/20121026. The second regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5c82f5a5e90d&tochange=5ecff3e46ed5

Comment 3

[Cristian Nicolae (:xti)]

This crash occurred on the latest Nightly just after I tried to open a website from the Top Sites on about:home:

https://crash-stats.mozilla.com/report/index/bp-6eb1a842-7445-434b-8899-617372121030

--
Firefox 19.0a1 (2012-10-29)
Device: Galaxy R
OS: Android 2.3.4

Comment 4

[Benoit Girard (:BenWa)]

This looks to be related to bug 795234.

Comment 5

[Benoit Girard (:BenWa)]

Actually if that's the case then this crash should stop now that bug 795259 is landed. That would be 2012-10-26 or later.

Comment 6

[Scoobidiver (away)]

(In reply to Benoit Girard (:BenWa) from comment #5)
> That would be 2012-10-26 or later.
That's not the case. There are crashes in the latest Nightly, about 15 per build since it appeared.

Comment 7

[Benoit Girard (:BenWa)]

I'm curious what view your using to see 15 per build? In the table it looks like a few crashes per build.

In any case you're right. I'm trying to see what we're crashing. As I understand it this feature should be turned off so likely it's only half off and crashing when something tries to call it. Should be easy fix if that's the case.

Comment 8

[Benoit Girard (:BenWa)]

I added an abort in TakeScreenshot and it is indeed getting called on startup:
I/GeckoScreenshotHandler(30805): Screenshotting disabled
I/Gecko   (30805): SCREENSHOT?

Comment 9

[Scoobidiver (away)]

(In reply to Benoit Girard (:BenWa) from comment #7)
> I'm curious what view your using to see 15 per build?
The link at the bottom of comment 0.

Comment 10

[Benoit Girard (:BenWa)]

Ohh I though those were just crashes with similar addresses, not different builds.

Alright I confirmed that the crash is because we dispatch a checkerboard screenshot event before the preference is read that is likely the cause of this crash.

Comment 11

[Benoit Girard (:BenWa)]

Attachment: patch

This patch should fix this crash. The preference doesn't properly enable/disable the screenshot. We need to decide if we want to properly support the preference or remove the java screenshot. Until then this patch will fix the crash.

Comment 12

[Chris Lord [:cwiiis]]

(In reply to Benoit Girard (:BenWa) from comment #11)
> Created attachment 676893 [details] [diff] [review]
> patch
> 
> This patch should fix this crash. The preference doesn't properly
> enable/disable the screenshot. We need to decide if we want to properly
> support the preference or remove the java screenshot. Until then this patch
> will fix the crash.

it should, but we really shouldn't crash even as it is now - I think we should take this patch anyway, as it'll be a startup improvement (I think?)

Perhaps the Java code assumes ownership when it shouldn't, or perhaps freeing the buffer while there's a screenshot message in flight causes this crash? Maybe it could be better fixed by making sure not to do any screenshotting until the pref read is returned?

Comment 13

[Benoit Girard (:BenWa)]

(In reply to Chris Lord [:cwiiis] from comment #12)
> Perhaps the Java code assumes ownership when it shouldn't, or perhaps
> freeing the buffer while there's a screenshot message in flight causes this
> crash? Maybe it could be better fixed by making sure not to do any
> screenshotting until the pref read is returned?

I think what happens is we free the buffer when we disable the sceenshot but don't check if we have a message queued and when we try to use it we crash. This looks like a race condition that depends in the timing of event execution so you may or may not crash.

The preference is broken, I looked brief at fixing it but the code calling it makes some bad assumption that makes it non trivial to fix. I say we fix this crash since the preference is broken to begin with and we file a follow up bug to either (1) remove the code, (2) fix the preference.

Comment 14

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Benoit Girard (:BenWa) from comment #13)
> (In reply to Chris Lord [:cwiiis] from comment #12)
> > Perhaps the Java code assumes ownership when it shouldn't, or perhaps
> > freeing the buffer while there's a screenshot message in flight causes this
> > crash? Maybe it could be better fixed by making sure not to do any
> > screenshotting until the pref read is returned?
> 
> I think what happens is we free the buffer when we disable the sceenshot but
> don't check if we have a message queued and when we try to use it we crash.
> This looks like a race condition that depends in the timing of event
> execution so you may or may not crash.

That was the cause of bug 795234 but that should be fixed now. If not, I'd like to get a proper fix.

> 
> The preference is broken, I looked brief at fixing it but the code calling
> it makes some bad assumption that makes it non trivial to fix. I say we fix
> this crash since the preference is broken to begin with and we file a follow
> up bug to either (1) remove the code, (2) fix the preference.

How soon will we have low-res tiles? if it is relatively soon, I'd be ok with taking this patch and a follow up to remove the code.

Comment 15

[Benoit Girard (:BenWa)]

I don't think it will be this cycle. We will likely use a bit of time to stabilize progressive drawing (regressions and crashes) and that will put us at the end of a cycle to drop a major feature.

Comment 16

[Brad Lassey [:blassey] (use needinfo?)]

that's fine, but in this case let's take this bandaid and fix the race condition that it is papering over in case we need to turn screenshots back on at some point

Comment 17

[Benoit Girard (:BenWa)]

Comment on attachment 676893 [details] [diff] [review]
patch

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

Comment 18

[Benoit Girard (:BenWa)]

Filed bug 807775 to fix the preference.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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