889231 - [Bluetooth] Lock-up occurs when you make a call in no signal network state

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect, P2)

Description

[leo.bugzilla.gecko]

Dear Mozilla Team,

I found the serious bug in BluetoothRilListener.cpp.

If you make a call in no signal network state, HandleCallStateChanged method is run always because NotifyError() is run of BluetoothRilListener.

At this time, it try to disconnect despite the non-call status and AudioFlinger is abnormal as below log.

01-13 14:30:09.989 I       588      Gecko                  -*- QCContentHelper_QC_B2G: sendMessage to content process: RIL:CallError{ callIndex : -1, errorMsg : 'BadNumberError',  }
01-13 14:30:09.999 I       588      Gecko                  -*- RILContentHelper: Received message 'RIL:CallError': {"callIndex":-1,"errorMsg":"BadNumberError"}
01-13 14:30:09.999 I       588      Gecko                  -*- RILContentHelper: callback handler for notifyError threw an exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIRILTelephonyCallback.notifyError]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js :: _deliverCallback :: line 1290"  data: no]

01-13 14:30:10.009 V       588      BluetoothHfpManager    HandleCallStateChanged:1351 [LGBT_gecko] aCallIndex = -1, aCallState = 10, aSend = 1

01-13 14:30:10.049 V       587      AudioFlinger           BUFFER TIMEOUT: remove(4097) from active list on thread 0x1876208
01-13 14:30:12.399 V       587      AudioFlinger           Audio hardware entering standby, mixer 0x1876208, mSuspended 0
01-13 14:30:12.399 I       587      AudioHardwareMSM76XXA  set device to SND_DEVICE_SPEAKER device_id=0
01-13 14:30:12.399 V       587      AudioHardwareMSM76XXA  MBADRC and ADRC Disabled
01-13 14:30:12.399 I       587      AudioHardwareMSM76XXA  disabling post proc features with mask 0x0020
01-13 14:30:12.399 D       587      AudioHardwareMSM76XXA  Enable SRS flags=0xff state= 0
01-13 14:30:12.409 V       587      AudioFlinger           MixerThread 0x1876208 TID 596 going to sleep

I think that it need a exception handling for HandleCallStateChanged in NotifyError() method.

Please check this.
Thanks.

Comment 1

[Ben Tian (inactive)]

Attachment: patch 1: v1: Filter out invalid call index number for HandleCallStateChanged

The patch filters out invalid call index number before hfp->HandleCallStateChanged() is called in NotifyError().

Comment 2

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

Comment on attachment 770088 [details] [diff] [review]
patch 1:  v1: Filter out invalid call index number for HandleCallStateChanged

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

Looks good. Although I'm not 100% sure about what the actual problem is from the reporter's description, I think this patch does the right thing.

Comment 3

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

birch: https://hg.mozilla.org/projects/birch/rev/f0a3fca09674

Comment 4

[leo.bugzilla.gecko]

(In reply to Eric Chou [:ericchou] [:echou] from comment #2)
> Comment on attachment 770088 [details] [diff] [review]
patch 1:  v1: Filter
> out invalid call index number for HandleCallStateChanged

Review of
> attachment 770088 [details] [diff] [review]:
> -----------------------------------------------------------------

Looks
> good. Although I'm not 100% sure about what the actual problem is from the
> reporter's description, I think this patch does the right thing.

Hi Eric,

This bug is reproduced as below step.
1. BT headset connect with b2g phone.
2. Try to outgoing call when a 3G network signal lost
3. b2g phone is stuck about 1 minute.
4. After 1 minute, phone reset.

In step2, If a call is released abnormally, then notifyError() will be called, instead of callStateChanged(). So hfp->HandleCallStateChanged is always called for CALL_STATE_DISCONNECTED because notifyError() is called in BluetoothRilListener.cpp

Comment 5

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

Hi Leo,

> 
> This bug is reproduced as below step.
> 1. BT headset connect with b2g phone.
> 2. Try to outgoing call when a 3G network signal lost
> 3. b2g phone is stuck about 1 minute.
> 4. After 1 minute, phone reset.
> 

Thanks for clarification.

> In step2, If a call is released abnormally, then notifyError() will be
> called, instead of callStateChanged(). So hfp->HandleCallStateChanged is
> always called for CALL_STATE_DISCONNECTED because notifyError() is called in
> BluetoothRilListener.cpp

We just confirmed with Hsin-yi, peer of RIL, about how to deal with NotifyError(). AFAIK, if:

(1) the callIndex is valid (>= 1)

Then we believe that the call has been dropped abnormally, so changing the state to CALL_STATE_DISCONNECTED should be correct.

(2) the callIndex is invalid (< 1, must be -1 with current RIL implementation)

No need to handle it since the call object is not even created.

That's why I think this patch should work. It would be great if you could try with applying this patch before we close this issue.

Comment 6

[leo.bugzilla.gecko]

Hi Eric,

You are right. No action is needed when callIndex is invalid.
I applied the patch and it is working well.

Thanks for your help.

Comment 7

[Ben Tian (inactive)]

Attachment: patch 1: b2g18 specific patch, r=echou, a=leo+

*b2g18 specific patch.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/1815a8d12429

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/1815a8d12429