commit | 2a3b9b597ee07f8728bd9903b0bc104c4e24fd6c | [log] [tgz] |
---|---|---|
author | Pavlin Radoslavov <pavlin@google.com> | Mon Apr 30 13:39:20 2018 -0700 |
committer | Myles Watson <mylesgw@google.com> | Mon Apr 30 22:01:52 2018 +0000 |
tree | 3772a733c1a1b61a5a7e0eae816de60324a67227 | |
parent | 2bc59e595600c8058e0e8afe16b2602c03baba55 [diff] |
Fix A2DP Suspend related multi-component deadlock Apparently, the processing of A2DP Suspend from the Audio HAL could result in a deadlock across multiple components: A2dpService -> AudioService -> Audio-BT HAL -> BT Native Stack -> BT JNI upcall -> A2dpService 1) A2dpService -> AudioService: - Waiting inside setActiveDevice() -> synchronized (mStateMachines) -> mAudioManager.setBluetoothA2dpDeviceConnectionStateSuppressNoisyIntent() - The mAudioManager call can be blocking: a followup call would wait for the previous call to complete. 2) AudioService -> Audio-BT HAL The AudioService is wating on the Audio-BT HAL to complete its request. In this case the request is SUSPEND 3) Audio-BT HAL -> BT Native Stack The Audio side of the Audio-BT HAL is waiting for the BT Native stack to ACK the SUSPEND command. More specifically, it is retrying few times with timeout of 250ms each iteration. 4) BT Native Stack -> BT JNI upcall - In the BT Native Stack the BtifAv FSM (see file btif/src/btif_av.cc) is in state StateStarted and has received the BTIF_AV_SUSPEND_STREAM_REQ_EVT because of the SUSPEND command from the Audio HAL. - As a result, the LocalSuspendPending flag has been set: peer_.SetFlags(BtifAvPeer::kFlagLocalSuspendPending); - Only after the above flag is reset, then the BT Native Stack will ACK the SUSPEND command from the HAL - The above flag will be reset when event BTA_AV_SUSPEND_EVT is received from the underlying stack - The BtifAv state machine has received the BTA_AV_SUSPEND_EVT event. However, prior to clearing the pending status, the FSM tries to report the audio state via JNI to the Java layer: btif_report_audio_state(); ... peer_.ClearFlags(BtifAvPeer::kFlagLocalSuspendPending); - Apparently, all the processing inside the BtifAv FSM is done on the JNI thread, including the btif_report_audio_state() upcall. 5) BT JNI upcall -> A2dpService - The btif_report_audio_state() upcall calls indirectly A2dpNativeInterface.onAudioStateChanged() -> sendMessageToService() -> A2dpService.messageFromNative() - Within the A2dpService.messageFromNative() processing, there is "synchronized (mStateMachines) {}" block, and the processing there blocks because per (1) above A2dpService.setActiveDevice() is already inside its own "synchronized (mStateMachines) {}" block The above deadlock is fixed by the following: - Modified the BTA_AV_SUSPEND_EVT processing such that the clearing of the kFlagLocalSuspendPending flag is done BEFORE the btif_report_audio_state() - Modified the BtifAv FSM such that the internal processing of the FSM events is done on the BTA thread instead of the JNI thread. Thus, the FSM processing cannot be blocked by JNI upcalls. Also: - Updated the do_in_bta_thread() function to have a return value so it is more aligned with the corresponding do_in_jni_thread() implementation - Updated the reordering of some other btif_report_*() upcalls inside btif_av.cc so they are after the FSM internal processing in the current state. - Added few extra logs to the Audio-BT HAL (file audio_a2dp_hw.cc). Bug: 72823323 Test: Disconnect/reconnect with Philips SBH7000 Headset Change-Id: I7ccb558e458a8e90d0414f94463bda8d4b3fdc97
Just build AOSP - Fluoride is there by default.
Instructions for Ubuntu, tested on 14.04 with Clang 3.5.0 and 16.10 with Clang 3.8.0
mkdir ~/fluoride cd ~/fluoride git clone https://android.googlesource.com/platform/system/bt
Install dependencies (require sudo access):
cd ~/fluoride/bt build/install_deps.sh
Then fetch third party dependencies:
cd ~/fluoride/bt mkdir third_party cd third_party git clone https://github.com/google/googletest.git git clone https://android.googlesource.com/platform/external/aac git clone https://android.googlesource.com/platform/external/libchrome git clone https://android.googlesource.com/platform/external/libldac git clone https://android.googlesource.com/platform/external/modp_b64 git clone https://android.googlesource.com/platform/external/tinyxml2
And third party dependencies of third party dependencies:
cd fluoride/bt/third_party/libchrome/base/third_party mkdir valgrind cd valgrind curl https://chromium.googlesource.com/chromium/src/base/+/master/third_party/valgrind/valgrind.h?format=TEXT | base64 -d > valgrind.h curl https://chromium.googlesource.com/chromium/src/base/+/master/third_party/valgrind/memcheck.h?format=TEXT | base64 -d > memcheck.h
NOTE: If system/bt is checked out under AOSP, then create symbolic links instead of downloading sources
cd system/bt mkdir third_party cd third_party ln -s ../../../external/aac aac ln -s ../../../external/libchrome libchrome ln -s ../../../external/libldac libldac ln -s ../../../external/modp_b64 modp_b64 ln -s ../../../external/tinyxml2 tinyxml2 ln -s ../../../external/googletest googletest
cd ~/fluoride/bt gn gen out/Default
cd ~/fluoride/bt ninja -C out/Default all
This will build all targets (the shared library, executables, tests, etc) and put them in out/Default. To build an individual target, replace "all" with the target of your choice, e.g. ninja -C out/Default net_test_osi
.
cd ~/fluoride/bt/out/Default LD_LIBRARY_PATH=./ ./bluetoothtbd -create-ipc-socket=fluoride
Follows the Chromium project Eclipse Setup Instructions until "Optional: Building inside Eclipse" section (don't do that section, we will set it up differently)
Generate Eclipse settings:
cd system/bt gn gen --ide=eclipse out/Default
In Eclipse, do File->Import->C/C++->C/C++ Project Settings, choose the XML location under system/bt/out/Default
Right click on the project. Go to Preferences->C/C++ Build->Builder Settings. Uncheck "Use default build command", but instead using "ninja -C out/Default"
Goto Behaviour tab, change clean command to "-t clean"