On Tue, 12 Apr 2016 18:46:17 +0200, Lars Lindqvist wrote:
Den 2016-04-12 skrev Takashi Iwai:
Actually we have a semaphore before shm access, so the race at two opens shouldn't happen. I noticed it after I sent my previous mail.
But the semaphore is taken also at snd_pcm_dmix_close(). So I wonder where the race actually happens. Both open and close must be protected while another stream is opening or closing.
Could you try to check where you get the exact error...?
The execution tree, as far as I can find is the following:
snd_pcm_dmix_open:
- Line 1009 snd_pcm_direct_shm_create_or_connect with the code in question,
- which returns 0. So we end up in line 1058, dmix->shmtr->use_server is 0,
- so go to line 1072.. running:
snd_pcm_open_slave, running: snd_pcm_open_named_slave, running: snd_pcm_open_conf
- where snd_dlobj_cache_get gives open_func = snd_pcm_hw_open, so
snd_pcm_hw_open() snd_open_device("/dev/snd/pcmC0D0p")
- where open() returns -1 with errno = EBADFD
OK, then the question is why other stream could be closed while this is being opened and protected via semaphore. Maybe the semaphore protection isn't perfect?
In anyway, in such a case, we may retry opening the stream as the first element. This is safer than blindly assuming the first element via nattach value (which is racy). An untested patch is below.
thanks,
Takashi
--- diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c index b26a5c790e7e..007d35664ce7 100644 --- a/src/pcm/pcm_dmix.c +++ b/src/pcm/pcm_dmix.c @@ -1020,6 +1020,7 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name, dmix->max_periods = opts->max_periods; dmix->sync_ptr = snd_pcm_dmix_sync_ptr;
+ retry: if (first_instance) { /* recursion is already checked in snd_pcm_direct_get_slave_ipc_offset() */ @@ -1076,6 +1077,13 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name, SND_PCM_APPEND, NULL); if (ret < 0) { + /* all other streams have been closed; + * retry as the first instance + */ + if (ret == -EBADFD) { + first_instance = 1; + goto retry; + } SNDERR("unable to open slave"); goto _err; }