1142132 - BluetoothHandsfreeResultHandler, void>::~BluetoothHALInterfaceRunnable0() - Device crashes when connecting to a bluetooth headset or speaker device.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Martin Shuman [:Marty]]

Attachment: logcat-bluetooth-crash.txt

BluetoothHandsfreeResultHandler, void>::~BluetoothHALInterfaceRunnable0() | mozilla::dom::bluetooth::BluetoothHALInterfaceRunnable0<mozilla::dom::bluetooth::Bcrash in @0x0 | mozilla::dom::bluetooth::BluetoothHALInterfaceRunnable0<mozilla::dom::bluetooth::

This bug was filed from the Socorro interface and is 
report bp-c8be5926-29ca-498a-b6d1-7ae1e2150311.
=============================================================

Description:
Connecting the device to a bluetooth headset or speaker will cause the phone to crash and restart.

The phone is able to pair with these devices, but crashes when trying to connect.

Repro Steps:
1) Update a Flame to 20150311010231
2) Open the Settings app and enable bluetooth
3) Pair and connect the phone with a bluetooth headset or speaker

Actual:
Phone crashes and restarts when connecting to the device.

Expected:
The phone connects properly to the bluetooth device.

Environmental Variables:
Device: Flame 3.0 (319MB)(Full Flash)
Build ID: 20150311010231
Gaia: 943c8b4039f59b08ba100390e164a076a20c892e
Gecko: fd8e079d6335
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Repro frequency: 8/8
Link to failed test case: https://moztrap.mozilla.org/manage/case/6070/
See attached: logcat, video (URL)

Comment 1

[Martin Shuman [:Marty]]

This issue does NOT occur on Flame 2.2.
The phone connects properly to the bluetooth device.

Environmental Variables:
Device: Flame 2.2 (319MB)(Full Flash)
Build ID: 20150311002522
Gaia: 3f032238a52f08e4c2f68a47ad065a96eb22d470
Gecko: 004fa1cb1dd4
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 2

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Functional Regression for a core feature that fails smoketest.

Requesting a window.

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20150310043519
Gaia: 63737c8d053bcbccf58d34dc5586e9d7f1d3d63f
Gecko: f481ba01e113
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

First Broken Environmental Variables:
Device: Flame
BuildID: 20150310055522
Gaia: 63737c8d053bcbccf58d34dc5586e9d7f1d3d63f
Gecko: f0e9898a7604
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=f481ba01e113&tochange=f0e9898a7604

Caused by patches for Bug 1137151.

Comment 4

[KTucker [:KTucker][Inactive 3/4/2016]]

Thomas, can you take a look at this? Looks like the cause is the landing for bug 1137151. This is a smoketest blocker. Can we get this backed out?

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

We should try backing out of https://bug1137151.bugzilla.mozilla.org/attachment.cgi?id=8571324 specifically.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Referring to :
https://hg.mozilla.org/mozilla-central/rev/d645da56de29

Comment 7

[No-Jun Park [:njpark]]

Ah, this fails because of Bug 1139987.  Didn't know there are test scripts actually look for the status bar info.

Comment 8

[No-Jun Park [:njpark]]

(In reply to No-Jun Park [:njpark] from comment #7)
> Ah, this fails because of Bug 1139987.  Didn't know there are test scripts
> actually look for the status bar info.

Sorry, wrong bug - please disregard

Comment 9

[bhavana bajaj [:bajaj]]

nhirata, did a local backout of https://hg.mozilla.org/mozilla-central/rev/d645da56de29 and his own build to verify smoketest around bluetooth work after this backout and we are good to go with backing this rev from his end

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Backout listed here : 
http://hg.mozilla.org/mozilla-central/rev/c33922ee3ac3

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Resolved by backout.

Comment 12

[Peter Bylenga [:PBylenga]]

Adding qawanted to verify on next available nightly.

Comment 13

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

The backout push broke ICS emulator builds:
https://treeherder.mozilla.org/logviewer.html#?job_id=1150744&repo=mozilla-central

So I backed out the backout in https://hg.mozilla.org/mozilla-central/rev/e1262bdb5b14

Comment 14

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Seems like there's still more that we have to backout with destructors and such.  Lessons learned; we probably need to push to try as well.

Comment 15

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: bt_backout.patch

Backout patch to back out:
d5e5fcb29452
d645da56de29

Comment 16

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [01] Bug 1142132: Move helper classes of HFP manager into manager's namespace

This bug is yet another instance of the symbol-collision problem that we've seen several times before. Shawn, is a smoke-test blocker. Can you give some priority to the review? Thanks!

Comment 17

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [02] Bug 1142132: Move helper classes of A2DP manager into manager's namespace

Comment 18

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: [03] Bug 1142132: Move helper classes of OPP manager into manager's namespace

Comment 19

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8576598 [details] [diff] [review]
[02] Bug 1142132: Move helper classes of A2DP manager into manager's namespace

Review of attachment 8576598 [details] [diff] [review]:
-----------------------------------------------------------------

r=me

Comment 20

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8576597 [details] [diff] [review]
[01] Bug 1142132: Move helper classes of HFP manager into manager's namespace

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

Bug 1005934

User impact if declined: 

Undefined behavior of the b2g process. Can also crash.

Testing completed: 

I locally tested by performing the STR.

Risk to taking this patch (and alternatives if risky): 

Small. The code is the same as before and the patches only 'rename' some of the C++ symbols by moving class declarations to other namespaces.

String or UUID changes made by this patch:

None

Comment 21

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8576598 [details] [diff] [review]
[02] Bug 1142132: Move helper classes of A2DP manager into manager's namespace

[Approval Request Comment]

Please see comment 20.

Comment 22

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8576599 [details] [diff] [review]
[03] Bug 1142132: Move helper classes of OPP manager into manager's namespace

[Approval Request Comment]

Please see comment 20.

Comment 23

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Thanks Shawn!

https://hg.mozilla.org/integration/b2g-inbound/rev/2587c1af2ce0
https://hg.mozilla.org/integration/b2g-inbound/rev/68288ae86454
https://hg.mozilla.org/integration/b2g-inbound/rev/a7cb28e7abbd

https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=a7cb28e7abbd

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2587c1af2ce0
https://hg.mozilla.org/mozilla-central/rev/68288ae86454
https://hg.mozilla.org/mozilla-central/rev/a7cb28e7abbd

Comment 26

[No-Jun Park [:njpark]]

Tested the patch, it pairs/unpairs/re-pairs normally, and works well with the phone/music app.

Comment 27

[Martin Shuman [:Marty]]

This issue is Verified Fixed on Flame 3.0.
Bluetooth devices are able to successfully pair and connect to the device.

Environmental Variables:
Device: Flame 3.0 (319MB)(Full Flash)
Build ID: 20150313010238
Gaia: eabe35cf054d47087b37c1ca7db8143717fbd7f3
Gecko: 42afc7ef5ccb
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Comment 28

[bhavana bajaj [:bajaj]]

tdz, staus-2.2 is unaffected here so not sure why we need to uplift to b2g37, am i missing something?

Comment 29

[Thomas Zimmermann [:tzimmermann] [:tdz]]

(In reply to bhavana bajaj [:bajaj] from comment #28)
> tdz, staus-2.2 is unaffected here so not sure why we need to uplift to
> b2g37, am i missing something?

This bug is an issue with the C++ linker, which cannot detect if two C++ classes classes of the same name are really the same implementation. So it picks one and throws away the other; even if the classes are not the same. The result is undefined.

The patch that made the bug visible is actually completely unrelated to the problem. Next time we add a patch to Bluetooth on v2.2, we could trigger the issue again. So I thought it makes sense to uplift these patches into v2.2.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

(In reply to bhavana bajaj [:bajaj] from comment #28)
> tdz, staus-2.2 is unaffected here so not sure why we need to uplift to
> b2g37, am i missing something?

Also a good way to make sure that the bug is invisible to the "needs uplift" queries.

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/07cc117f1319
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/aebe8488986e
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/3fac93f8276b