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