Re: [PATCH RFC alsa-lib 1/5] pcm: Add thread-safety to PCM API

[Clemens Ladisch <clemens@ladisch.de>]

Takashi Iwai wrote:
> --- a/src/pcm/pcm.c
> +++ b/src/pcm/pcm.c
> @@ -686,6 +686,8 @@ snd_pcm_stream_t snd_pcm_stream(snd_pcm_t *pcm)
>   *
>   * Closes the specified PCM handle and frees all associated
>   * resources.
> + *
> + * The function is thread-safe when built with the proper option.
>   */
>  int snd_pcm_close(snd_pcm_t *pcm)
>  {
> @@ -697,6 +699,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
>  		if (err < 0)
>  			res = err;
>  	}
> +	snd_pcm_lock(pcm);
>  	if (pcm->mmap_channels)
>  		snd_pcm_munmap(pcm);
>  	while (!list_empty(&pcm->async_handlers)) {
> @@ -704,6 +707,7 @@ int snd_pcm_close(snd_pcm_t *pcm)
>  		snd_async_del_handler(h);
>  	}
>  	err = pcm->ops->close(pcm->op_arg);
> +	snd_pcm_unlock(pcm);
>  	if (err < 0)
>  		res = err;
>  	err = snd_pcm_free(pcm);

Thread safety does not really make sense for a destructor like
snd_pcm_close().  If any other code is accessing the device at the same
time, the freeing will make it crash anyway.

What would make sense would be to output a debug warning if the device
is locked, and/or to poison the device (maybe ->ops = NULL).


Regards,
Clemens