974629 - crash in mozilla::layers::TextureClient::Finalize()

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Natalya Kot [:nkot]]

This bug was filed from the Socorro interface and is 
report bp-61506b98-eb0e-4de6-8ed0-05a0c2140219.
=============================================================

Description:
Hit this crash a few times while running manual smoketests. 
Unable to provide reliable STR, crash happened while running through FTE and while sending multiple SMSs.

STR I:
1. Open Messages app
2. Send multiple SMSs to any contact 
3. Send SMS to the same contact from Contacts app (Contact details, chat icon)

STR II:
1. Reset device from Settings to run FTE
2. Hit Next until "About Firefox OS" screen when the user needs to enter email
3. Tap in the entry field and type in any char
4. Hit "Done" button
5. Hit "Back" button
6. Repeat 4-6 a few times

Actual:
app crash occurs

Expected:
no crashes

Buri master build
BuildID: 20140219040204
Gaia: ac06cfbd2baf6494ffbb668cc599e3892cd5e17b
Gecko: bf0e76f2a7d4
Version: 30.0a1
v1.2-devices.cfg

Comment 1

[Matt Woodrow (:mattwoodrow)]

https://crash-stats.mozilla.com/report/index/66fee3f6-38c5-420c-9f8f-7852f2140221

Comment 2

[Natalya Kot [:nkot]]

actually, STR II from comment 0 can serve as valid steps, the crash happens quite often when running through FTE and attempting to type in and submit email address - repro rate 5/10

So please try:
STR II:
1. Reset device from Settings to run FTE
2. Hit Next until "About Firefox OS" screen when the user needs to enter email
3. Type in email address or just a few chars 
4. Hit "Done" button
==> often, crash  occurs once started typing in any chars, sometimes when submitted the email by pressing "Done"

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Natalya Kot [:nkot] from comment #0)
> 
> STR II:
> 1. Reset device from Settings to run FTE
> 2. Hit Next until "About Firefox OS" screen when the user needs to enter
> email
> 3. Tap in the entry field and type in any char
> 4. Hit "Done" button
> 5. Hit "Back" button
> 6. Repeat 4-6 a few times

Natalya, can you explain about step 4 more? When I did step 4, FTU moved to "Start your phone tour!" and there is no "Back" button. Only "Skip" and "Start tour" buttons.

Comment 4

[Natalya Kot [:nkot]]

(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> 
> Natalya, can you explain about step 4 more? When I did step 4, FTU moved to
> "Start your phone tour!" and there is no "Back" button. Only "Skip" and
> "Start tour" buttons.

When you first open About Firefox OS screen with the email entry field, down below there are "Back" and "Done" buttons (for the user to choose it they want to go back to FTE or to complete FTE). Once pressed "Done" the user us taken to "Start tour" if NO crash happened!

So, the key point here, once the user start typing in any chars, or even a single one, or full email address into the entry field, and when they either tap "Back" or "Done" it will cause this crash to happen.  

It's not 100% repro, but happens to us very often.

Comment 5

[Natalya Kot [:nkot]]

Attachment: logcat

had a chance to grab a logcat, at this time I used different STR:

STR:
1. Have two contacts on the test device (A, B)
2. Open Messages and send SMS to contact A
3. Open Dialer/Contacts and send SMS to contact B
4. Open Messages again and send another SMS
5. Might need to repeat steps 2-4
==> crash occurs

Comment 6

[Sotaro Ikeda [:sotaro]]

I still can not reproduce it. But I found one possibility from the source code. TextureClient::Finalize() uses the Forwarder like the following. But the pointer is set by TextureClient::InitIPDLActor() call. If it was not called because of some reasons. The crash seems to happen.

>     actor->GetForwarder()->RemoveTexture(this);

And all crash cases include ImageContainer in the crash log. If ImageContainer is used for rendering, TextureClient::InitIPDLActor() is not called until, ImageClientSingle::UpdateImageInternal() call for rendering. Therefore it seems possible.

Comment 7

[Sotaro Ikeda [:sotaro]]

Attachment: patch_974629_1

Comment 8

[Sotaro Ikeda [:sotaro]]

https://tbpl.mozilla.org/?tree=Try&rev=99178b854442

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

Note that the crash address in comment 0 is far from null, so the present crash is not fixable by just a null pointer check.

Of course, it could still be the case that a null pointer check is useful here.

I just hit apparently the same crash on desktop with tabs.remote (desktop IPC). Attaching a GDB session...

Comment 10

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: GDB session showing dead mForwarder in the TextureChild

See how this GDB session shows that actor->GetForwarder() is non-null, but pointing to an already dead CompositableForwarder.

Comment 11

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Make TextureChild hold on to its CompositableForwarder

Seems to fix it for me... (was reproducible on desktop by setting remote IPC, viewing a page with a Flash video, and quitting the browser while it's playing).

Comment 12

[Sotaro Ikeda [:sotaro]]

Benoit, thanks for the information! You are correct. From the crash log, the crash in Comment 0, seems to happen by accessing dead CompositableForwarder. So, it seems like to related to application's shut down problem.

By the benoit's comment, the problem seems to be fixed by changed the raw pointer to ref counted one.

Comment 13

[Sotaro Ikeda [:sotaro]]

It might be better also checking the patch made by bjacob on tryserver.
https://tbpl.mozilla.org/?tree=Try&rev=8ab4ec7cef92

Comment 14

[Benoit Jacob [:bjacob] (mostly away)]

Thanks for the try run!

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e560b97c43eb

Comment 16

[Sotaro Ikeda [:sotaro]]

https://hg.mozilla.org/integration/b2g-inbound/rev/4e8a51fc68b0

Comment 17

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

https://hg.mozilla.org/mozilla-central/rev/4e8a51fc68b0

Comment 19

[Carsten Book [:Tomcat]]

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