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

[Takashi Iwai <tiwai@suse.de>]

On Tue, 05 Jul 2016 18:30:36 +0200,
Clemens Ladisch wrote:
> 
> 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.

That's true.  OTOH, snd_pcm_close() does sync before closing, so I
thought it might be not too bad to be prepared for bad programmers.
But I'm fine to get rid of it.

> 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).

Yes, more debugging option would be nicer.  We can add it later.


thanks,

Takashi