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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Greg Grisco]

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)

Comment 1

[Greg Grisco]

In the logs I see:

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

after which in the code sBtInterface is set to null

Comment 2

[pgravel]

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.

Comment 3

[bhavana bajaj [:bajaj]]

NI Shawn to help investigate this.

Comment 4

[Greg Grisco]

sorry, comment 1 was meant for another bug.

Comment 5

[Ken Chang[:kenkai]]

ni shawn

Comment 6

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

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|.

Comment 7

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

I cannot hit this bug on flame-kk/nexus-5. But i can reproduce this bug on device 'Orion'.

Comment 8

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

Attachment: bug1152095.patch

Comment 9

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

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.

Comment 10

[Keven Kuo [:kkuo]]

Comment on attachment 8589601 [details] [diff] [review]
bug1152095.patch

[Triage Comment]FC blocker

Comment 11

[Inder]

nick -- please land this patch in our build.

Comment 12

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

Attachment: Bug 1152095 - Check mResultHandlerQ is empty before dispatching DispatchError, r=tzimmermann

Comment 13

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

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

Comment 14

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

(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. :(

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/72297a30f2d4

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/72297a30f2d4

Comment 17

[Ryan VanderMeulen [:RyanVM]]

I think this needs b2g37 approval from RelMan first.

Comment 18

[bhavana bajaj [:bajaj]]

can land now, same comment https://bugzilla.mozilla.org/show_bug.cgi?id=1149616#c13 applies here.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/bd28d0ec0f4e