audio_a2dp_hw: add device lock
Prevent conflict between closing a stream (adev_close_output_stream)
and setting parameters (adev_set_parameters).
Test: Manual - Bluetooth A2DP streaming and changing codec parameters
Bug: 36723276
Change-Id: Id8b9fcdf594e3d5fde139719d324420468c4c0aa
(cherry picked from commit b1a900fcc9903f0988b6f8c1a470b9c718aedf1f)
diff --git a/audio_a2dp_hw/audio_a2dp_hw.cc b/audio_a2dp_hw/audio_a2dp_hw.cc
index 24a5014..2a0c7e0 100644
--- a/audio_a2dp_hw/audio_a2dp_hw.cc
+++ b/audio_a2dp_hw/audio_a2dp_hw.cc
@@ -95,7 +95,10 @@
struct a2dp_stream_out;
struct a2dp_audio_device {
+ // Important: device must be first as an audio_hw_device* may be cast to
+ // a2dp_audio_device* when the type is implicitly known.
struct audio_hw_device device;
+ std::recursive_mutex* mutex; // See note below on mutex acquisition order.
struct a2dp_stream_in* input;
struct a2dp_stream_out* output;
};
@@ -109,7 +112,7 @@
/* move ctrl_fd outside output stream and keep open until HAL unloaded ? */
struct a2dp_stream_common {
- std::recursive_mutex* mutex;
+ std::recursive_mutex* mutex; // See note below on mutex acquisition order.
int ctrl_fd;
int audio_fd;
size_t buffer_sz;
@@ -129,6 +132,15 @@
struct a2dp_stream_common common;
};
+/*
+ * Mutex acquisition order:
+ *
+ * The a2dp_audio_device (adev) mutex must be acquired before
+ * the a2dp_stream_common (out or in) mutex.
+ *
+ * This may differ from other audio HALs.
+ */
+
/*****************************************************************************
* Static variables
*****************************************************************************/
@@ -1354,7 +1366,8 @@
int ret = 0;
INFO("opening output");
-
+ // protect against adev->output and stream_out from being inconsistent
+ std::lock_guard<std::recursive_mutex> lock(*a2dp_dev->mutex);
out = (struct a2dp_stream_out*)calloc(1, sizeof(struct a2dp_stream_out));
if (!out) return -ENOMEM;
@@ -1454,17 +1467,21 @@
struct a2dp_audio_device* a2dp_dev = (struct a2dp_audio_device*)dev;
struct a2dp_stream_out* out = (struct a2dp_stream_out*)stream;
- INFO("closing output (state %d)", out->common.state);
+ // prevent interference with adev_set_parameters.
+ std::lock_guard<std::recursive_mutex> lock(*a2dp_dev->mutex);
+ {
+ std::lock_guard<std::recursive_mutex> lock(*out->common.mutex);
+ const a2dp_state_t state = out->common.state;
+ INFO("closing output (state %d)", (int)state);
+ if ((state == AUDIO_A2DP_STATE_STARTED) ||
+ (state == AUDIO_A2DP_STATE_STOPPING)) {
+ stop_audio_datapath(&out->common);
+ }
- std::unique_lock<std::recursive_mutex> lock(*out->common.mutex);
- if ((out->common.state == AUDIO_A2DP_STATE_STARTED) ||
- (out->common.state == AUDIO_A2DP_STATE_STOPPING)) {
- stop_audio_datapath(&out->common);
+ skt_disconnect(out->common.ctrl_fd);
+ out->common.ctrl_fd = AUDIO_SKT_DISCONNECTED;
}
- skt_disconnect(out->common.ctrl_fd);
- out->common.ctrl_fd = AUDIO_SKT_DISCONNECTED;
- lock.unlock();
a2dp_stream_common_destroy(&out->common);
free(stream);
a2dp_dev->output = NULL;
@@ -1475,9 +1492,12 @@
static int adev_set_parameters(struct audio_hw_device* dev,
const char* kvpairs) {
struct a2dp_audio_device* a2dp_dev = (struct a2dp_audio_device*)dev;
- struct a2dp_stream_out* out = a2dp_dev->output;
int retval = 0;
+ // prevent interference with adev_close_output_stream
+ std::lock_guard<std::recursive_mutex> lock(*a2dp_dev->mutex);
+ struct a2dp_stream_out* out = a2dp_dev->output;
+
if (out == NULL) return retval;
INFO("state %d", out->common.state);
@@ -1562,6 +1582,8 @@
FNLOG();
+ // protect against adev->input and stream_in from being inconsistent
+ std::lock_guard<std::recursive_mutex> lock(*a2dp_dev->mutex);
in = (struct a2dp_stream_in*)calloc(1, sizeof(struct a2dp_stream_in));
if (!in) return -ENOMEM;
@@ -1617,16 +1639,20 @@
struct audio_stream_in* stream) {
struct a2dp_audio_device* a2dp_dev = (struct a2dp_audio_device*)dev;
struct a2dp_stream_in* in = (struct a2dp_stream_in*)stream;
- a2dp_state_t state = in->common.state;
- INFO("closing input (state %d)", state);
+ std::lock_guard<std::recursive_mutex> lock(*a2dp_dev->mutex);
+ {
+ std::lock_guard<std::recursive_mutex> lock(*in->common.mutex);
+ const a2dp_state_t state = in->common.state;
+ INFO("closing input (state %d)", (int)state);
- if ((state == AUDIO_A2DP_STATE_STARTED) ||
- (state == AUDIO_A2DP_STATE_STOPPING))
- stop_audio_datapath(&in->common);
+ if ((state == AUDIO_A2DP_STATE_STARTED) ||
+ (state == AUDIO_A2DP_STATE_STOPPING))
+ stop_audio_datapath(&in->common);
- skt_disconnect(in->common.ctrl_fd);
- in->common.ctrl_fd = AUDIO_SKT_DISCONNECTED;
+ skt_disconnect(in->common.ctrl_fd);
+ in->common.ctrl_fd = AUDIO_SKT_DISCONNECTED;
+ }
a2dp_stream_common_destroy(&in->common);
free(stream);
a2dp_dev->input = NULL;
@@ -1642,8 +1668,11 @@
}
static int adev_close(hw_device_t* device) {
+ struct a2dp_audio_device* a2dp_dev = (struct a2dp_audio_device*)device;
FNLOG();
+ delete a2dp_dev->mutex;
+ a2dp_dev->mutex = nullptr;
free(device);
return 0;
}
@@ -1664,6 +1693,8 @@
if (!adev) return -ENOMEM;
+ adev->mutex = new std::recursive_mutex;
+
adev->device.common.tag = HARDWARE_DEVICE_TAG;
adev->device.common.version = AUDIO_DEVICE_API_VERSION_2_0;
adev->device.common.module = (struct hw_module_t*)module;
