Last Comment Bug 782905 - [b2g-bluetooth] Calling setEnabled(false) twice in a very short period may crash
: [b2g-bluetooth] Calling setEnabled(false) twice in a very short period may crash
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Eric Chou [:ericchou] [:echou]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-15 03:00 PDT by Eric Chou [:ericchou] [:echou]
Modified: 2012-08-16 06:17 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1: Check if mBluetoothCommandThread is null (913 bytes, patch)
2012-08-15 03:15 PDT, Eric Chou [:ericchou] [:echou]
kyle: review+
Details | Diff | Splinter Review

Description Eric Chou [:ericchou] [:echou] 2012-08-15 03:00:26 PDT
Evelyn found a bug: If BluetoothManager.setEnabled(false) is called twice almost at the same time, it may lead to system crash.

I've checked and found the root cause. In class ToggleBtAck, we try to shutdown gBluetoothService->mBluetoothCommandThread. When the second setEnabled(false) is called, mBluetoothCommandThread may have been swapped by the first setEnabled, which means that gBluetoothService->mBluetoothCommandThread will be null.

The easiest and efficient way to solve this is to add an if-statement to ensure mBluetoothCommandThread is not null.
Comment 1 Eric Chou [:ericchou] [:echou] 2012-08-15 03:15:28 PDT
Created attachment 652056 [details] [diff] [review]
v1: Check if mBluetoothCommandThread is null
Comment 2 Kyle Machulis [:kmachulis] [:qdot] 2012-08-15 11:59:04 PDT
Comment on attachment 652056 [details] [diff] [review]
v1: Check if mBluetoothCommandThread is null

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

r=me, I think this should work, though we should probably have bent take a look at this before it lands since this is totally a regression I caused when we landed 768306, where I removed the reference counting. :/
Comment 3 Kyle Machulis [:kmachulis] [:qdot] 2012-08-15 12:03:05 PDT
Comment on attachment 652056 [details] [diff] [review]
v1: Check if mBluetoothCommandThread is null

Ok, bent's out this week, and I'm mostly sure this should work at least for the moment. Will file a followup to talk to bent about this when he gets back. Landable for now.
Comment 4 Eric Chou [:ericchou] [:echou] 2012-08-15 19:49:18 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb192b83831c
Comment 5 Ed Morley [:emorley] 2012-08-16 06:17:25 PDT
https://hg.mozilla.org/mozilla-central/rev/cb192b83831c

Note You need to log in before you can comment on or make changes to this bug.