[alsa-devel] EBADFD caused by commit dec428c352217010e4b8bd750d302b8062339d32

[Takashi Iwai]

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;
 			}