Re: [PATCH - pcm 1/1] pcm: fix "unable to create IPC shm instance" in some case

[Takashi Iwai <tiwai@suse.de>]

On Thu, 03 Mar 2016 13:40:42 +0100,
IceBsi wrote:
> 
> From: Qing Cai <caiqing@neusoft.com>
> 
> As stated in manpage SHMCTL(2), shm_nattch is "No. of current attaches"
> (i.e., number of processes attached to the shared memeory). If an
> application uses alsa-lib and invokes fork() at some point, there should
> be the following execution sequence:
>  1. execute the following statement:
>       pcm_direct.c:110: dmix->shmptr = shmat(dmix->shmid, 0, 0)
>     (shm_nattch becomes 1)
>  2. invoke fork() in some thread.
>     (shm_nattch becomes 2)
>  3. execute the following statement:
>       pcm_direct.c:122: if (buf.shm_nattch == 1)
>  4. execute the following statement:
>       pcm_direct.c:131: if (dmix->shmptr->magic != SND_PCM_DIRECT_MAGIC)
>     (As stated in manpage SHMGET(2), "When a new shared memory segment
>      is created, its contents are initialized to zero values", so
>      dmix->shmptr->magic is 0)
>  5. execute the following statements:
>       pcm_direct.c:132: snd_pcm_direct_shm_discard(dmix)
>       pcm_direct.c:133: return -EINVAL
> The above execution sequence will cause the following error:
>   unable to create IPC shm instance
> This error causes multimedia application has no sound. This error rarely
> occurs, probability is about 1%.
> Because the first user of the shared memory will get that
> dmix->shmptr->magic is 0, check dmix->shmptr->magic's value to determine
> if "we're the first user" is OK.
> Tests have been made 400+ times after this fix, and the issue no longer
> exists.

I think this is still racy.  Multiple users can grab the shmem at the
very same time.  Maybe it looks as if working just because both users
behavior as the first user and do clear and initialize.

The check of bus.shm_nattach=1 should be fine, per se.  The problem is
the magic key check of the secondary.  In the current code, as you
pointed out, this may happen before the first client finishes the
initialization.

One option would be to use some lock like pthread mutex.  Another
option would be to spin for a while until it gets a non-zero value.
I guess the latter is more suitable for dmix & co.

Thoughts?


thanks,

Takashi


> 
> Signed-off-by: Qing Cai <bsiice@msn.com>
> Signed-off-by: Qing Cai <caiqing@neusoft.com>
> 
> diff --git a/src/pcm/pcm_direct.c b/src/pcm/pcm_direct.c
> index fd3877c..93d6f3c 100644
> --- a/src/pcm/pcm_direct.c
> +++ b/src/pcm/pcm_direct.c
> @@ -119,7 +119,7 @@ retryget:
>  		snd_pcm_direct_shm_discard(dmix);
>  		return err;
>  	}
> -	if (buf.shm_nattch == 1) {	/* we're the first user, clear the segment */
> +	if (dmix->shmptr->magic != SND_PCM_DIRECT_MAGIC) {	/* we're the first user, clear the segment */
>  		memset(dmix->shmptr, 0, sizeof(snd_pcm_direct_share_t));
>  		if (dmix->ipc_gid >= 0) {
>  			buf.shm_perm.gid = dmix->ipc_gid;
> -- 
> 2.7.2
>