[vlc-commits] auhal: rework locking and avoid potential deadlock
David Fuhrmann
git at videolan.org
Sat Aug 2 22:29:52 CEST 2014
vlc/vlc-2.2 | branch: master | David Fuhrmann <dfuhrmann at videolan.org> | Wed Jul 30 18:45:04 2014 +0200| [cca33e15fcac5a7d597eacdb2084beca4983fb69] | committer: Jean-Baptiste Kempf
auhal: rework locking and avoid potential deadlock
hopefully closes #11675
(cherry picked from commit 2034ba88d6e1779f6717a16520d1a8b07759b2aa)
Signed-off-by: Jean-Baptiste Kempf <jb at videolan.org>
> http://git.videolan.org/gitweb.cgi/vlc/vlc-2.2.git/?a=commit;h=cca33e15fcac5a7d597eacdb2084beca4983fb69
---
modules/audio_output/auhal.c | 72 +++++++++++++++++++++++++++---------------
1 file changed, 47 insertions(+), 25 deletions(-)
diff --git a/modules/audio_output/auhal.c b/modules/audio_output/auhal.c
index 6253052..9bdd4fd 100644
--- a/modules/audio_output/auhal.c
+++ b/modules/audio_output/auhal.c
@@ -106,8 +106,12 @@ struct aout_sys_t
int i_bytes_per_sample;
CFArrayRef device_list;
+ vlc_mutex_t device_list_lock; /* protects access to device_list */
- vlc_mutex_t var_lock; /* protects access to device_list and i_selected_dev */
+ vlc_mutex_t selected_device_lock;/* Synchronizes access to i_selected_dev. This is only needed
+ between VLCs audio thread and the core audio callback thread.
+ The value is only changed in Start, further access to this variable
+ within the audio thread (start, stop, close) needs no protection. */
float f_volume;
bool b_mute;
@@ -186,7 +190,8 @@ static int Open(vlc_object_t *obj)
OSStatus err = noErr;
- vlc_mutex_init(&p_sys->var_lock);
+ vlc_mutex_init(&p_sys->device_list_lock);
+ vlc_mutex_init(&p_sys->selected_device_lock);
vlc_mutex_init(&p_sys->lock);
vlc_cond_init(&p_sys->cond);
p_sys->b_digital = false;
@@ -263,13 +268,21 @@ static void Close(vlc_object_t *obj)
if (err != noErr)
msg_Err(p_aout, "failed to remove listener for default audio device [%4.4s]", (char *)&err);
- vlc_mutex_lock(&p_sys->var_lock);
+ /*
+ * StreamsChangedListener can rebuild the device list and thus held the device_list_lock.
+ * To avoid a possible deadlock, an array copy is created here.
+ * In rare cases, this can lead to missing StreamsChangedListener callback deregistration (TODO).
+ */
+ vlc_mutex_lock(&p_sys->device_list_lock);
+ CFArrayRef device_list_cpy = CFArrayCreateCopy(NULL, p_sys->device_list);
+ vlc_mutex_unlock(&p_sys->device_list_lock);
+
/* remove streams callbacks */
- CFIndex count = CFArrayGetCount(p_sys->device_list);
+ CFIndex count = CFArrayGetCount(device_list_cpy);
if (count > 0) {
for (CFIndex x = 0; x < count; x++) {
AudioDeviceID deviceId = 0;
- CFNumberRef cfn_device_id = CFArrayGetValueAtIndex(p_sys->device_list, x);
+ CFNumberRef cfn_device_id = CFArrayGetValueAtIndex(device_list_cpy, x);
if (!cfn_device_id)
continue;
@@ -280,14 +293,15 @@ static void Close(vlc_object_t *obj)
}
}
+ CFRelease(device_list_cpy);
CFRelease(p_sys->device_list);
- vlc_mutex_unlock(&p_sys->var_lock);
char *psz_device = aout_DeviceGet(p_aout);
config_PutPsz(p_aout, "auhal-audio-device", psz_device);
free(psz_device);
- vlc_mutex_destroy(&p_sys->var_lock);
+ vlc_mutex_destroy(&p_sys->selected_device_lock);
+ vlc_mutex_destroy(&p_sys->device_list_lock);
vlc_mutex_destroy(&p_sys->lock);
vlc_cond_destroy(&p_sys->cond);
@@ -318,7 +332,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
p_sys->b_paused = false;
p_sys->i_device_latency = 0;
- vlc_mutex_lock(&p_sys->var_lock);
+ vlc_mutex_lock(&p_sys->selected_device_lock);
p_sys->i_selected_dev = p_sys->i_new_selected_dev;
aout_FormatPrint(p_aout, "VLC is looking for:", fmt);
@@ -365,7 +379,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
err = AudioObjectGetPropertyData(kAudioObjectSystemObject, &defaultDeviceAddress, 0, NULL, &propertySize, &defaultDeviceID);
if (err != noErr) {
msg_Err(p_aout, "could not get default audio device [%4.4s]", (char *)&err);
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
goto error;
}
else
@@ -374,7 +388,7 @@ static int Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
p_sys->i_selected_dev = defaultDeviceID;
p_sys->b_selected_dev_is_digital = var_InheritBool(p_aout, "spdif");
}
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
// recheck if device still supports digital
b_start_digital = p_sys->b_selected_dev_is_digital;
@@ -1286,7 +1300,7 @@ static void RebuildDeviceList(audio_output_t * p_aout)
free(psz_name);
}
- vlc_mutex_lock(&p_sys->var_lock);
+ vlc_mutex_lock(&p_sys->device_list_lock);
CFIndex count = 0;
if (p_sys->device_list)
count = CFArrayGetCount(p_sys->device_list);
@@ -1302,6 +1316,7 @@ static void RebuildDeviceList(audio_output_t * p_aout)
if (cfn_device_id) {
CFNumberGetValue(cfn_device_id, kCFNumberSInt32Type, &i_device_id);
msg_Dbg(p_aout, "Device ID %i is not found in new array, deleting.", i_device_id);
+
ReportDevice(p_aout, i_device_id, NULL);
}
}
@@ -1311,7 +1326,7 @@ static void RebuildDeviceList(audio_output_t * p_aout)
CFRelease(p_sys->device_list);
p_sys->device_list = CFArrayCreateCopy(kCFAllocatorDefault, currentListOfDevices);
CFRelease(currentListOfDevices);
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->device_list_lock);
free(deviceIDs);
}
@@ -1582,12 +1597,14 @@ static OSStatus DevicesListener(AudioObjectID inObjectID, UInt32 inNumberAddres
msg_Dbg(p_aout, "audio device configuration changed, resetting cache");
RebuildDeviceList(p_aout);
- vlc_mutex_lock(&p_sys->var_lock);
+ vlc_mutex_lock(&p_sys->selected_device_lock);
+ vlc_mutex_lock(&p_sys->device_list_lock);
CFNumberRef selectedDevice = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &p_sys->i_selected_dev);
if(!CFArrayContainsValue(p_sys->device_list, CFRangeMake(0, CFArrayGetCount(p_sys->device_list)), selectedDevice))
aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT);
CFRelease(selectedDevice);
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->device_list_lock);
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
return noErr;
}
@@ -1624,6 +1641,8 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i
if (!p_aout)
return -1;
+ aout_sys_t *p_sys = p_aout->sys;
+
if (!p_aout->sys->b_selected_dev_is_default)
return noErr;
@@ -1646,12 +1665,13 @@ static OSStatus DefaultDeviceChangedListener(AudioObjectID inObjectID, UInt32 i
return noErr;
}
+ vlc_mutex_lock(&p_sys->selected_device_lock);
/* Also ignore events which announce the same device id */
- if(defaultDeviceID == p_aout->sys->i_selected_dev)
- return noErr;
-
- msg_Dbg(p_aout, "default device actually changed, resetting aout");
- aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT);
+ if(defaultDeviceID != p_aout->sys->i_selected_dev) {
+ msg_Dbg(p_aout, "default device actually changed, resetting aout");
+ aout_RestartRequest(p_aout, AOUT_RESTART_OUTPUT);
+ }
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
return noErr;
}
@@ -1680,37 +1700,39 @@ static OSStatus StreamsChangedListener(AudioObjectID inObjectID, UInt32 inNumbe
msg_Dbg(p_aout, "available physical formats for audio device changed");
RebuildDeviceList(p_aout);
+ vlc_mutex_lock(&p_sys->selected_device_lock);
/* In this case audio has not yet started. Below code will not work and is not needed here. */
- if (p_sys->i_selected_dev == 0)
+ if (p_sys->i_selected_dev == 0) {
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
return 0;
+ }
/*
* check if changed stream id belongs to current device
*/
- vlc_mutex_lock(&p_sys->var_lock);
AudioObjectPropertyAddress streamsAddress = { kAudioDevicePropertyStreams, kAudioDevicePropertyScopeOutput, kAudioObjectPropertyElementMaster };
err = AudioObjectGetPropertyDataSize(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size);
if (err != noErr) {
msg_Err(p_aout, "could not get number of streams for device %i [%4.4s]", p_sys->i_selected_dev, (char *)&err);
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
return VLC_EGENERIC;
}
i_streams = i_param_size / sizeof(AudioStreamID);
p_streams = (AudioStreamID *)malloc(i_param_size);
if (p_streams == NULL) {
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
return VLC_ENOMEM;
}
err = AudioObjectGetPropertyData(p_sys->i_selected_dev, &streamsAddress, 0, NULL, &i_param_size, p_streams);
if (err != noErr) {
msg_Err(p_aout, "could not get list of streams [%4.4s]", (char *)&err);
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
free(p_streams);
return VLC_EGENERIC;
}
- vlc_mutex_unlock(&p_sys->var_lock);
+ vlc_mutex_unlock(&p_sys->selected_device_lock);
for (int i = 0; i < i_streams; i++) {
if (p_streams[i] == inObjectID) {
More information about the vlc-commits
mailing list