Closed Bug 1152095 Opened 5 years ago Closed 5 years ago

Crash in bluetooth while quickly toggling BT icon in notification window with bluetoothd running

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
critical

Tracking

(blocking-b2g:2.2+, firefox38 wontfix, firefox39 wontfix, firefox40 fixed, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S10 (17apr)
blocking-b2g 2.2+
Tracking Status
firefox38 --- wontfix
firefox39 --- wontfix
firefox40 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: ggrisco, Assigned: shawnjohnjr)

References

Details

(Keywords: crash, Whiteboard: [caf priority: p2][CR 819586])

Attachments

(2 files)

Steps to reproduce:

1. Go to settings/bluetooth
2. Pull down the notification window
3. Quickly toggle the bluetooth icon at bottom of screen (faster is better)

100% reproducible crash:

#0  0xb6da27d2 in memmove (dst0=0xb66ce0b0 <mozilla::CountingAllocatorBase<NesteggReporter>::sAmount>, src0=<optimized out>, length=4294967292)
    at bionic/libc/upstream-openbsd/lib/libc/string/bcopy.c:97
#1  0xb5613d8a in nsTArray_Impl<nsRefPtr<mozilla::dom::bluetooth::BluetoothResultHandler>, nsTArrayInfallibleAllocator>::RemoveElementsAt (this=this@entry=0xb17bfe20, 
    aStart=aStart@entry=0, aCount=aCount@entry=1) at ../../dist/include/nsTArray.h:1398
#2  0xb5613dd2 in RemoveElementAt (aIndex=0, this=0xb17bfe20) at ../../dist/include/nsTArray.h:1403
#3  mozilla::dom::bluetooth::BluetoothDaemonInterface::OnConnectError (this=this@entry=0xb17bfe00, 
    aChannel=aChannel@entry=mozilla::dom::bluetooth::BluetoothDaemonInterface::CMD_CHANNEL)
    at ../../../../../../../../gecko/dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp:1882
#4  0xb56141b0 in mozilla::dom::bluetooth::BluetoothDaemonInterface::Init (this=0xb17bfe00, aNotificationHandler=<optimized out>, aRes=0xaea5f2b0)
    at ../../../../../../../../gecko/dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp:2121
#5  0xb561affa in mozilla::dom::bluetooth::BluetoothServiceBluedroid::StartGonkBluetooth ()
    at ../../../../../../../../gecko/dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp:307
#6  0xb561b046 in mozilla::dom::bluetooth::BluetoothServiceBluedroid::StartInternal (this=<optimized out>)
    at ../../../../../../../../gecko/dom/bluetooth/bluedroid/BluetoothServiceBluedroid.cpp:401
#7  0xb5608a10 in mozilla::dom::bluetooth::BluetoothService::StartBluetooth (this=this@entry=0xb17bfdc0, aIsStartup=aIsStartup@entry=false)
    at ../../../../../../../../gecko/dom/bluetooth/BluetoothService.cpp:397
#8  0xb5608cca in mozilla::dom::bluetooth::BluetoothService::StartStopBluetooth (this=this@entry=0xb17bfdc0, aStart=<optimized out>, aIsStartup=aIsStartup@entry=false)
In the logs I see:

I/GeckoBluetooth(  261): OnError: BluetoothInterface::Init failed: 5

after which in the code sBtInterface is set to null
In BluetoothDaemonInterface::OnConnectError, ElementAt(0) is used without checking if it actually exists, which appears to be causing this crash.

Notes: There is a IsEmpty() earlier on in the function, but it doesn't do anything since MOZ_ASSERTs are not enabled.
NI Shawn to help investigate this.
sorry, comment 1 was meant for another bug.
ni shawn
Flags: needinfo?(shuang)
Assignee: nobody → shuang
Flags: needinfo?(shuang)
AFAICT We can fix that by simply checking for IsEmpty() in the |OnConnectError| method.

When quickly toggling Bluetooth, I guess that we switch off bluetoothd, but it doesn't have time to shutdown cleanly. So we kill it in |BluetoothDaemonInterface::Init|.
I cannot hit this bug on flame-kk/nexus-5. But i can reproduce this bug on device 'Orion'.
Comment on attachment 8589601 [details] [diff] [review]
bug1152095.patch

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

Looks good, thanks Shawn. Just out of curiosity, did you find out how exactly this gets triggered?

::: dom/bluetooth/bluedroid/BluetoothDaemonInterface.cpp
@@ +1890,2 @@
>        break;
>    }

I guess you can remove the extra brackets.
Attachment #8589601 - Flags: review?(tzimmermann) → review+
blocking-b2g: 2.2? → 2.2+
Comment on attachment 8589601 [details] [diff] [review]
bug1152095.patch

[Triage Comment]FC blocker
Attachment #8589601 - Flags: approval-mozilla-b2g37+
Whiteboard: [CR 819586]
Whiteboard: [CR 819586] → [caf priority: p2][CR 819586]
nick -- please land this patch in our build.
Flags: needinfo?(ntroast)
Comment on attachment 8590202 [details] [diff] [review]
Bug 1152095 - Check mResultHandlerQ is empty before dispatching DispatchError, r=tzimmermann

[Triage Comment]FC blocker
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 1073548
User impact if declined: Crash
Testing completed: I cannot reproduce this bug on flame-kk
Risk to taking this patch (and alternatives if risky): Just add check
String or UUID changes made by this patch: None
Attachment #8590202 - Flags: approval-mozilla-b2g37?
Attachment #8590202 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #9)
> Comment on attachment 8589601 [details] [diff] [review]
> bug1152095.patch
> 
> Review of attachment 8589601 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, thanks Shawn. Just out of curiosity, did you find out how
> exactly this gets triggered?
Not really, I got some trouble to reproduce this bug when attaching gdb or getting logcat. :(
Flags: needinfo?(ntroast)
https://hg.mozilla.org/mozilla-central/rev/72297a30f2d4
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S10 (17apr)
I think this needs b2g37 approval from RelMan first.
Flags: needinfo?(bbajaj)
can land now, same comment https://bugzilla.mozilla.org/show_bug.cgi?id=1149616#c13 applies here.
Flags: needinfo?(bbajaj) → needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.