823203 - Pulling your headphones out of the phone as soon as the music app loads causes a crash

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Cody Roesch [:croesch]]

Attachment: log.txt

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121128204232

Steps to reproduce:

1. Launch Build 20121217070202 Version 18.0
2. Plug in your headphones into the phone.
3. Make sure Music App is not running in the background already. (Important!)
3. Launch the Music app and immediately pull the headphones out of the phone as the Music app is loading.
4. Phone will crash.

Repro 100% if pulled as soon as music app is launched.


Actual results:

Phone crashes when the headphones are pulled when the music app is launched.

Also I am able to crash the phone by plugging in or pulling the headphones out at the home screen but that is not 100% so I figured i'd write up the 100% version in hopes you can find the culprit for a blanket fix.


Expected results:

The phone should be recognize the state of the headphones but should not crash when the state changes.

Comment 1

[Marcia Knous [:marcia]]

I have seen this bug intermittently, probably because of Step #3.

Comment 2

[Robert Kaiser]

Do you have any crash IDs from reports you submitted?

Comment 3

[Cody Roesch [:croesch]]

The Crash ID: 1355958864

Comment 4

[Marcia Knous [:marcia]]

The crash ID should look something like this: https://crash-stats.mozilla.com/report/index/fca56a80-ae69-41d3-9b51-909b92121219

(In reply to croesch from comment #3)
> The Crash ID: 1355958864

Comment 5

[Robert Kaiser]

You can get a list of crash IDs for your device via this adb command:

adb shell ls -l /data/b2g/mozilla/Crash\ Reports/submitted/

Comment 6

[Marcia Knous [:marcia]]

I was able to reproduce this with my headphones, but not with the ones that come with the phone. Working on getting a crash ID.

Comment 7

[Cody Roesch [:croesch]]

Attachment: Crash ID

Comment 8

[Robert Kaiser]

Just mentioning bp-072f36ac-502d-4c44-a309-c34032121220 here would have been enough, but thanks. :)
Judging from how the stack there looks, I think this is affected by bug 822432 comment #18, I hope we can fix that up soon.

Comment 9

[Andrew Overholt [:overholt]]

Marco, can you take this?  mwu also suggested rlin as a possible owner.

Comment 10

[Robert Kaiser]

(In reply to Robert Kaiser (:[e-mail]) from comment #8)
> Just mentioning bp-072f36ac-502d-4c44-a309-c34032121220 here would have been
> enough, but thanks. :)

OK, this one now has a usable stack, adding the signature here.

Comment 11

[Marco Chen [:mchen]]

Yes, I can take it.

Comment 12

[Marco Chen [:mchen]]

Hi croesch,

1. I found that this bug can't be reproduced in the first time of launching music app.
2. I can crash the device always by following steps
  a. To launch the music app.
  b. To remove music app from card view.
  c. plug or pull out the headset.
  d. then device is crashed.

Could you help to confirm this is the same with what you reported?
It seems to that someone didn't unregister it's observer from b2g process when process is killed by card view.

Thanks.

Comment 13

[Marco Chen [:mchen]]

Attachment: Patch v1

1. Music App will register an observer for switch device via IPDL to b2g process.
2. When music app is killed by card view, b2g process didn't receive unregister of observer. So there is an illegal pointer of observer now.
3. Then b2g process will crash when plugging/pulling out headset in this situation.

Solution: Add unregister codes into HalParent::ActorDestroy().

Comment 14

[Marco Chen [:mchen]]

Comment on attachment 694738 [details] [diff] [review]
Patch v1

Hi Justin,

Thanks for your review in advance.

Comment 15

[Marco Chen [:mchen]]

Comment on attachment 694738 [details] [diff] [review]
Patch v1

Change the reviewer to Chris Jones because Justin Lebar noted he is away from 12/21.

Hi Chris,

Thanks for review.

Comment 16

[Justin Lebar (not reading bugmail)]

Comment on attachment 694738 [details] [diff] [review]
Patch v1

r=me.  If you haven't checked this already, can you check that we're not missing any other Unregister calls?

Comment 17

[Marco Chen [:mchen]]

Attachment: Patch Checkin-Version

There are 7 items which used register observers from child to parent.
And all of them will call unregister observers from HalParent::ActorDestroy except the switch observer in this bug.

7 items now: battery, network, ScreenConfiguration, sys clock, sys time, sensor, wakelock and switch.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e52fedf18088
https://hg.mozilla.org/releases/mozilla-b2g18/rev/1cf3bb21595c

Comment 22

[John Shih]

verified unagi
build info
gaia master: 1be7bf421a6498f6e73377e3028227dea99b3431
b2g-18: e261861b0270

Comment 23

[Cody Roesch [:croesch]]

Verified as fixed in build 20121231070201.