Fix semaphore deadlocks

- Fixed unbalanced semaphores (which may cause deadlock)
- Do semaphore-up before blocking calls for communication with the server
- Don't discard semaphores on the client side
- Open slave PCMs always in non-blocking mode to avoid blocking by semaphore
  with the secondary open

diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
index 14e4b8a5db6e2419516d37edbc46871e5ab0020b..a0f88da01de47b8a38a0baafceb317dd32228412 100644 (file)
--- a/src/pcm/pcm_direct.c
+++ b/src/pcm/pcm_direct.c
@@ -76,10 +76,8 @@ int snd_pcm_direct_semaphore_discard(snd_pcm_direct_t *dmix)
 
 int snd_pcm_direct_semaphore_down(snd_pcm_direct_t *dmix, int sem_num)
 {
-       struct sembuf op[2] = { { 0, 0, SEM_UNDO }, { 0, 1, SEM_UNDO | IPC_NOWAIT } };
+       struct sembuf op[2] = { { sem_num, 0, 0 }, { sem_num, 1, SEM_UNDO } };
        assert(dmix->semid >= 0);
-       op[0].sem_num = sem_num;
-       op[1].sem_num = sem_num;
        if (semop(dmix->semid, op, 2) < 0)
                return -errno;
        return 0;
@@ -87,9 +85,8 @@ int snd_pcm_direct_semaphore_down(snd_pcm_direct_t *dmix, int sem_num)
 
 int snd_pcm_direct_semaphore_up(snd_pcm_direct_t *dmix, int sem_num)
 {
-       struct sembuf op = { 0, -1, SEM_UNDO | IPC_NOWAIT };
+       struct sembuf op = { sem_num, -1, SEM_UNDO | IPC_NOWAIT };
        assert(dmix->semid >= 0);
-       op.sem_num = sem_num;
        if (semop(dmix->semid, &op, 1) < 0)
                return -errno;
        return 0;
@@ -252,6 +249,7 @@ static void server_job(snd_pcm_direct_t *dmix)
                        snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
                        if (shmctl(dmix->shmid, IPC_STAT, &buf) < 0) {
                                snd_pcm_direct_shm_discard(dmix);
+                               snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
                                continue;
                        }
                        server_printf("DIRECT SERVER: nattch = %i\n", (int)buf.shm_nattch);

diff --git a/src/pcm/pcm_dmix.c b/src/pcm/pcm_dmix.c
index 71ed8a9478a18b7898bf21041778e492c274e439..cca0cfa04ba83ff3e5008b3d60303cdc73c9178f 100644 (file)
--- a/src/pcm/pcm_dmix.c
+++ b/src/pcm/pcm_dmix.c
@@ -585,12 +585,8 @@ static int snd_pcm_dmix_close(snd_pcm_t *pcm)
        if (dmix->client)
                snd_pcm_direct_client_discard(dmix);
        shm_sum_discard(dmix);
-       if (snd_pcm_direct_shm_discard(dmix) > 0) {
-               if (snd_pcm_direct_semaphore_discard(dmix) < 0)
-                       snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
-       } else {
-               snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
-       }
+       snd_pcm_direct_shm_discard(dmix);
+       snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
        if (dmix->bindings)
                free(dmix->bindings);
        pcm->private_data = NULL;
@@ -748,12 +744,12 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
        dmix = calloc(1, sizeof(snd_pcm_direct_t));
        if (!dmix) {
                ret = -ENOMEM;
-               goto _err;
+               goto _err_nosem;
        }
        
        ret = snd_pcm_direct_parse_bindings(dmix, bindings);
        if (ret < 0)
-               goto _err;
+               goto _err_nosem;
        
        dmix->ipc_key = ipc_key;
        dmix->ipc_perm = ipc_perm;
@@ -769,13 +765,13 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
                ret = snd_pcm_direct_semaphore_create_or_connect(dmix);
                if (ret < 0) {
                        SNDERR("unable to create IPC semaphore");
-                       goto _err;
+                       goto _err_nosem;
                }
                ret = snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
                if (ret < 0) {
                        snd_pcm_direct_semaphore_discard(dmix);
                        if (--fail_sem_loop <= 0)
-                               goto _err;
+                               goto _err_nosem;
                        continue;
                }
                break;
@@ -795,7 +791,7 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
        dmix->sync_ptr = snd_pcm_dmix_sync_ptr;
 
        if (first_instance) {
-               ret = snd_pcm_open_slave(&spcm, root, sconf, stream, mode);
+               ret = snd_pcm_open_slave(&spcm, root, sconf, stream, mode | SND_PCM_NONBLOCK);
                if (ret < 0) {
                        SNDERR("unable to open slave");
                        goto _err;
@@ -825,12 +821,15 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
 
                dmix->shmptr->type = spcm->type;
        } else {
+               /* up semaphore to avoid deadlock */
+               snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
                ret = snd_pcm_direct_client_connect(dmix);
                if (ret < 0) {
                        SNDERR("unable to connect client");
-                       return ret;
+                       goto _err_nosem;
                }
                        
+               snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT);
                ret = snd_pcm_hw_open_fd(&spcm, "dmix_client", dmix->hw_fd, 0, 0);
                if (ret < 0) {
                        SNDERR("unable to open hardware");
@@ -883,27 +882,24 @@ int snd_pcm_dmix_open(snd_pcm_t **pcmp, const char *name,
        return 0;
        
  _err:
+       if (dmix->timer)
+               snd_timer_close(dmix->timer);
+       if (dmix->server)
+               snd_pcm_direct_server_discard(dmix);
+       if (dmix->client)
+               snd_pcm_direct_client_discard(dmix);
+       if (spcm)
+               snd_pcm_close(spcm);
+       if (dmix->u.dmix.shmid_sum >= 0)
+               shm_sum_discard(dmix);
+       if (dmix->shmid >= 0)
+               snd_pcm_direct_shm_discard(dmix);
+       if (snd_pcm_direct_semaphore_discard(dmix) < 0)
+               snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
+ _err_nosem:
        if (dmix) {
-               if (dmix->timer)
-                       snd_timer_close(dmix->timer);
-               if (dmix->server)
-                       snd_pcm_direct_server_discard(dmix);
-               if (dmix->client)
-                       snd_pcm_direct_client_discard(dmix);
-               if (spcm)
-                       snd_pcm_close(spcm);
-               if (dmix->u.dmix.shmid_sum >= 0)
-                       shm_sum_discard(dmix);
-               if (dmix->shmid >= 0) {
-                       if (snd_pcm_direct_shm_discard(dmix) > 0) {
-                               if (dmix->semid >= 0) {
-                                       if (snd_pcm_direct_semaphore_discard(dmix) < 0)
-                                               snd_pcm_direct_semaphore_up(dmix, DIRECT_IPC_SEM_CLIENT);
-                               }
-                       }
-               }
-               if (dmix->bindings)
-                       free(dmix->bindings);
+               if (dmix->bindings)
+                       free(dmix->bindings);
                free(dmix);
        }
        if (pcm)

diff --git a/src/pcm/pcm_dshare.c b/src/pcm/pcm_dshare.c
index 097e2ba6a319ebc88eae53fc8eefb20b9748309d..2ea977308ae010cdb9677b5116d17d6431c32809 100644 (file)
--- a/src/pcm/pcm_dshare.c
+++ b/src/pcm/pcm_dshare.c
@@ -459,12 +459,8 @@ static int snd_pcm_dshare_close(snd_pcm_t *pcm)
                snd_pcm_direct_server_discard(dshare);
        if (dshare->client)
                snd_pcm_direct_client_discard(dshare);
-       if (snd_pcm_direct_shm_discard(dshare) > 0) {
-               if (snd_pcm_direct_semaphore_discard(dshare) < 0)
-                       snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
-       } else {
-               snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
-       }
+       snd_pcm_direct_shm_discard(dshare);
+       snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
        if (dshare->bindings)
                free(dshare->bindings);
        pcm->private_data = NULL;
@@ -614,17 +610,17 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
        dshare = calloc(1, sizeof(snd_pcm_direct_t));
        if (!dshare) {
                ret = -ENOMEM;
-               goto _err;
+               goto _err_nosem;
        }
        
        ret = snd_pcm_direct_parse_bindings(dshare, bindings);
        if (ret < 0)
-               goto _err;
+               goto _err_nosem;
                
        if (!dshare->bindings) {
                SNDERR("dshare: specify bindings!!!");
                ret = -EINVAL;
-               goto _err;
+               goto _err_nosem;
        }
        
        dshare->ipc_key = ipc_key;
@@ -634,20 +630,20 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
 
        ret = snd_pcm_new(&pcm, dshare->type = SND_PCM_TYPE_DSHARE, name, stream, mode);
        if (ret < 0)
-               goto _err;
+               goto _err_nosem;
 
        while (1) {
                ret = snd_pcm_direct_semaphore_create_or_connect(dshare);
                if (ret < 0) {
                        SNDERR("unable to create IPC semaphore");
-                       goto _err;
+                       goto _err_nosem;
                }
        
                ret = snd_pcm_direct_semaphore_down(dshare, DIRECT_IPC_SEM_CLIENT);
                if (ret < 0) {
                        snd_pcm_direct_semaphore_discard(dshare);
                        if (--fail_sem_loop <= 0)
-                               goto _err;
+                               goto _err_nosem;
                        continue;
                }
                break;
@@ -667,7 +663,7 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
        dshare->sync_ptr = snd_pcm_dshare_sync_ptr;
 
        if (first_instance) {
-               ret = snd_pcm_open_slave(&spcm, root, sconf, stream, mode);
+               ret = snd_pcm_open_slave(&spcm, root, sconf, stream, mode | SND_PCM_NONBLOCK);
                if (ret < 0) {
                        SNDERR("unable to open slave");
                        goto _err;
@@ -694,12 +690,15 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
 
                dshare->shmptr->type = spcm->type;
        } else {
+               /* up semaphore to avoid deadlock */
+               snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
                ret = snd_pcm_direct_client_connect(dshare);
                if (ret < 0) {
                        SNDERR("unable to connect client");
-                       return ret;
+                       goto _err_nosem;
                }
                        
+               snd_pcm_direct_semaphore_down(dshare, DIRECT_IPC_SEM_CLIENT);
                ret = snd_pcm_hw_open_fd(&spcm, "dshare_client", dshare->hw_fd, 0, 0);
                if (ret < 0) {
                        SNDERR("unable to open hardware");
@@ -751,27 +750,24 @@ int snd_pcm_dshare_open(snd_pcm_t **pcmp, const char *name,
        return 0;
        
  _err:
+       if (dshare->shmptr)
+               dshare->shmptr->u.dshare.chn_mask &= ~dshare->u.dshare.chn_mask;
+       if (dshare->timer)
+               snd_timer_close(dshare->timer);
+       if (dshare->server)
+               snd_pcm_direct_server_discard(dshare);
+       if (dshare->client)
+               snd_pcm_direct_client_discard(dshare);
+       if (spcm)
+               snd_pcm_close(spcm);
+       if (dshare->shmid >= 0)
+               snd_pcm_direct_shm_discard(dshare);
+       if (snd_pcm_direct_semaphore_discard(dshare) < 0)
+               snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
+ _err_nosem:
        if (dshare) {
-               if (dshare->shmptr)
-                       dshare->shmptr->u.dshare.chn_mask &= ~dshare->u.dshare.chn_mask;
-               if (dshare->timer)
-                       snd_timer_close(dshare->timer);
-               if (dshare->server)
-                       snd_pcm_direct_server_discard(dshare);
-               if (dshare->client)
-                       snd_pcm_direct_client_discard(dshare);
-               if (spcm)
-                       snd_pcm_close(spcm);
-               if (dshare->shmid >= 0) {
-                       if (snd_pcm_direct_shm_discard(dshare) > 0) {
-                               if (dshare->semid >= 0) {
-                                       if (snd_pcm_direct_semaphore_discard(dshare) < 0)
-                                               snd_pcm_direct_semaphore_up(dshare, DIRECT_IPC_SEM_CLIENT);
-                               }
-                       }
-               }
-               if (dshare->bindings)
-                       free(dshare->bindings);
+               if (dshare->bindings)
+                       free(dshare->bindings);
                free(dshare);
        }
        if (pcm)

diff --git a/src/pcm/pcm_dsnoop.c b/src/pcm/pcm_dsnoop.c
index 73d0b6f142796b93e13d11f3ee36a00423c22cb6..02c71091feea60b99375fddc4017f5edbfd6b12e 100644 (file)
--- a/src/pcm/pcm_dsnoop.c
+++ b/src/pcm/pcm_dsnoop.c
@@ -366,12 +366,8 @@ static int snd_pcm_dsnoop_close(snd_pcm_t *pcm)
                snd_pcm_direct_server_discard(dsnoop);
        if (dsnoop->client)
                snd_pcm_direct_client_discard(dsnoop);
-       if (snd_pcm_direct_shm_discard(dsnoop) > 0) {
-               if (snd_pcm_direct_semaphore_discard(dsnoop) < 0)
-                       snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
-       } else {
-               snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
-       }
+       snd_pcm_direct_shm_discard(dsnoop);
+       snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
        if (dsnoop->bindings)
                free(dsnoop->bindings);
        pcm->private_data = NULL;
@@ -510,12 +506,12 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
        dsnoop = calloc(1, sizeof(snd_pcm_direct_t));
        if (!dsnoop) {
                ret = -ENOMEM;
-               goto _err;
+               goto _err_nosem;
        }
        
        ret = snd_pcm_direct_parse_bindings(dsnoop, bindings);
        if (ret < 0)
-               goto _err;
+               goto _err_nosem;
        
        dsnoop->ipc_key = ipc_key;
        dsnoop->ipc_perm = ipc_perm;
@@ -524,20 +520,20 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
 
        ret = snd_pcm_new(&pcm, dsnoop->type = SND_PCM_TYPE_DSNOOP, name, stream, mode);
        if (ret < 0)
-               goto _err;
+               goto _err_nosem;
 
        while (1) {
                ret = snd_pcm_direct_semaphore_create_or_connect(dsnoop);
                if (ret < 0) {
                        SNDERR("unable to create IPC semaphore");
-                       goto _err;
+                       goto _err_nosem;
                }
        
                ret = snd_pcm_direct_semaphore_down(dsnoop, DIRECT_IPC_SEM_CLIENT);
                if (ret < 0) {
                        snd_pcm_direct_semaphore_discard(dsnoop);
                        if (--fail_sem_loop <= 0)
-                               goto _err;
+                               goto _err_nosem;
                        continue;
                }
                break;
@@ -557,7 +553,7 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
        dsnoop->sync_ptr = snd_pcm_dsnoop_sync_ptr;
 
        if (first_instance) {
-               ret = snd_pcm_open_slave(&spcm, root, sconf, stream, mode);
+               ret = snd_pcm_open_slave(&spcm, root, sconf, stream, mode | SND_PCM_NONBLOCK);
                if (ret < 0) {
                        SNDERR("unable to open slave");
                        goto _err;
@@ -584,12 +580,15 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
 
                dsnoop->shmptr->type = spcm->type;
        } else {
+               /* up semaphore to avoid deadlock */
+               snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
                ret = snd_pcm_direct_client_connect(dsnoop);
                if (ret < 0) {
                        SNDERR("unable to connect client");
-                       return ret;
+                       goto _err_nosem;
                }
                        
+               snd_pcm_direct_semaphore_down(dsnoop, DIRECT_IPC_SEM_CLIENT);
                ret = snd_pcm_hw_open_fd(&spcm, "dsnoop_client", dsnoop->hw_fd, 0, 0);
                if (ret < 0) {
                        SNDERR("unable to open hardware");
@@ -634,25 +633,22 @@ int snd_pcm_dsnoop_open(snd_pcm_t **pcmp, const char *name,
        return 0;
        
  _err:
+       if (dsnoop->timer)
+               snd_timer_close(dsnoop->timer);
+       if (dsnoop->server)
+               snd_pcm_direct_server_discard(dsnoop);
+       if (dsnoop->client)
+               snd_pcm_direct_client_discard(dsnoop);
+       if (spcm)
+               snd_pcm_close(spcm);
+       if (dsnoop->shmid >= 0)
+               snd_pcm_direct_shm_discard(dsnoop);
+       if (snd_pcm_direct_semaphore_discard(dsnoop) < 0)
+               snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
+ _err_nosem:
        if (dsnoop) {
-               if (dsnoop->timer)
-                       snd_timer_close(dsnoop->timer);
-               if (dsnoop->server)
-                       snd_pcm_direct_server_discard(dsnoop);
-               if (dsnoop->client)
-                       snd_pcm_direct_client_discard(dsnoop);
-               if (spcm)
-                       snd_pcm_close(spcm);
-               if (dsnoop->shmid >= 0) {
-                       if (snd_pcm_direct_shm_discard(dsnoop) > 0) {
-                               if (dsnoop->semid >= 0) {
-                                       if (snd_pcm_direct_semaphore_discard(dsnoop) < 0)
-                                               snd_pcm_direct_semaphore_up(dsnoop, DIRECT_IPC_SEM_CLIENT);
-                               }
-                       }
-               }
-               if (dsnoop->bindings)
-                       free(dsnoop->bindings);
+               if (dsnoop->bindings)
+                       free(dsnoop->bindings);
                free(dsnoop);
        }
        if (pcm)