[alsa-devel] [PATCH - alsa-lib] pcm/ioplug: Add mutex in snd_pcm_ioplug_hw_ptr_update

[Takashi Iwai]

At Tue, 29 Oct 2013 15:52:16 +0530,
Vishal Agrawal wrote:
> 
> Function snd_pcm_ioplug_hw_ptr_update can be called from many threads, we
> need to protect the hw_ptr. I came across this issue with GStreamer and
> Jackd. Function will be called from the thread doing snd_pcm_write and
> another would be to get the playback position for the track bar.
> 
> Change in itself is very minimum, but it took me a lot of time to debug.

We'd like to avoid mutex in the alsa-lib code as much as possible.
In which code paths did you get the race?  In general, the code paths
involved with hw_ptr_update() are known to be thread-unsafe, so it's
the responsibility of the caller side to protect the race.


thanks,

Takashi

> 
> Thanks,
> Vishal Agrawal
> [1.2  <text/html; ISO-8859-1 (quoted-printable)>]
> 
> [2 0001-pcm-ioplug-Add-mutex-in-snd_pcm_ioplug_hw_ptr_update.patch <text/x-patch; US-ASCII (base64)>]
> From b826b1f28c50bd4a414cf50751be4ac74b4e4174 Mon Sep 17 00:00:00 2001
> From: Vishal Agrawal <visagrawal at gmail.com>
> Date: Mon, 28 Oct 2013 18:01:14 +0530
> Subject: [PATCH - alsa-lib] pcm/ioplug: Add mutex in
>  snd_pcm_ioplug_hw_ptr_update
> 
> snd_pcm_ioplug_hw_ptr_update() can be called from different
> threads, it needs to be protected by mutex
> 
> Signed-off-by: Vishal Agrawal <visagrawal at gmail.com>
> 
> diff --git a/src/pcm/pcm_ioplug.c b/src/pcm/pcm_ioplug.c
> index a90c844..23dbee3 100644
> --- a/src/pcm/pcm_ioplug.c
> +++ b/src/pcm/pcm_ioplug.c
> @@ -26,6 +26,7 @@
>   *
>   */
>    
> +#include <pthread.h>
>  #include "pcm_local.h"
>  #include "pcm_ioplug.h"
>  #include "pcm_ext_parm.h"
> @@ -47,12 +48,15 @@ typedef struct snd_pcm_ioplug_priv {
>  	snd_htimestamp_t trigger_tstamp;
>  } ioplug_priv_t;
>  
> +static pthread_mutex_t hw_ptr_mutex = PTHREAD_MUTEX_INITIALIZER;
> +
>  /* update the hw pointer */
>  static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm)
>  {
>  	ioplug_priv_t *io = pcm->private_data;
>  	snd_pcm_sframes_t hw;
>  
> +	pthread_mutex_lock(&hw_ptr_mutex);
>  	hw = io->data->callback->pointer(io->data);
>  	if (hw >= 0) {
>  		unsigned int delta;
> @@ -64,6 +68,7 @@ static void snd_pcm_ioplug_hw_ptr_update(snd_pcm_t *pcm)
>  		io->last_hw = hw;
>  	} else
>  		io->data->state = SNDRV_PCM_STATE_XRUN;
> +	pthread_mutex_unlock(&hw_ptr_mutex);
>  }
>  
>  static int snd_pcm_ioplug_info(snd_pcm_t *pcm, snd_pcm_info_t *info)
> -- 
> 1.7.9.5
>