1003661 - [Bluedroid] HfpManager and A2dpManager hit MOZ_ASSERT(aController && !mController) in Connect() and Disconnect() function during BT connection.

Status: RESOLVED DUPLICATE of bug 1003663

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Jamin Liu [:jaliu][:Jamin]]

STR:
1. Enable BT
2. Pair and connect to a A2DP and HFP device
3. Disable BT in the middle of connection procedure.
Repeat step 1-3 rapidly several times.

Comment 1

[Jamin Liu [:jaliu][:Jamin]]

Attachment: Remove unnecessary MOZ_ASSERT from BT profile connect/disconnect. (v0)

Comment 2

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

Comment on attachment 8415047 [details] [diff] [review]
Remove unnecessary MOZ_ASSERT from BT profile connect/disconnect. (v0)

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

My bad :(  This shall be caught in https://bugzilla.mozilla.org/show_bug.cgi?id=951634#c64, but somehow the final patch does not include it. When turning off Bluetooth, we disconnect all profiles before cutting off the power. In this case, no DOMRequest should be replied and we set aController to nullptr. Please check BluetoothService.cpp for further details. I forgot to add it in my final patch in bug 951634. Thanks!

Comment 3

[Eric Chou [:ericchou] [:echou]]

Comment on attachment 8415047 [details] [diff] [review]
Remove unnecessary MOZ_ASSERT from BT profile connect/disconnect. (v0)

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

Jamin,

The assertion "MOZ_ASSERT(mController)" which you removed from Bluetooth*Manager exists for a reason. We can only support one connection in each Bluetooth*Manager, otherwise data members such as mDeviceAddress and mConnected would not be trusted. I think we should figure out why the assertion is hit instead of removing the assertion. In addition, I notice that this bug might be fixed once bug 1003663 landed, so let's focus on 1003663 first.

Comment 4

[Jamin Liu [:jaliu][:Jamin]]

Can not reproduce this bug with https://hg.mozilla.org/integration/b2g-inbound/rev/5bc7a1c7054c
I'll mark it as DUPLICATE RESOLVED when Bug 1003663 is closed.

Comment 6

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

(In reply to Eric Chou [:ericchou] [:echou] from comment #3)
> Comment on attachment 8415047 [details] [diff] [review]
> Remove unnecessary MOZ_ASSERT from BT profile connect/disconnect. (v0)
> 
> Review of attachment 8415047 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Jamin,
> 
> The assertion "MOZ_ASSERT(mController)" which you removed from
> Bluetooth*Manager exists for a reason. We can only support one connection in
> each Bluetooth*Manager, otherwise data members such as mDeviceAddress and
> mConnected would not be trusted. I think we should figure out why the
> assertion is hit instead of removing the assertion. In addition, I notice
> that this bug might be fixed once bug 1003663 landed, so let's focus on
> 1003663 first.
https://bugzilla.mozilla.org/show_bug.cgi?id=951634#c4
This is what I saw:
a. HFP connected and A2DP is connecting
b. Bluetooth turned off during that time, A2DP is connecting
c. Bluetooth turned on, 
d. Bluetooth HFP is connecting but stuck
e. Turn off headset and turn on headset to reconnect to phone
f. HFP connected, and which trigger a2dp connection
g. Hit assertion due to previous A2DP outgoing connection was made and mController has been assigned.