[alsa-devel] Deadlock over semaphore issue with aplay while using dmix

[Takashi Iwai]

At Fri, 05 Apr 2013 17:43:41 +0200,
Jaroslav Kysela wrote:
> 
> Date 5.4.2013 17:27, Takashi Iwai wrote:
> > At Fri, 05 Apr 2013 17:19:52 +0200,
> > Jaroslav Kysela wrote:
> >>
> >> Date 4.4.2013 18:30, Takashi Iwai wrote:
> >>> At Thu, 04 Apr 2013 13:33:08 +0200,
> >>> Jaroslav Kysela wrote:
> >>>>
> >>>> Date 4.4.2013 11:27, mateen wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I seeing sometimes deadlock issue with dmix when I press CTRL+C.
> >>>>>
> >>>>> Aplay's signal handler calls snd_pcm_close() if an interrupt occurs.
> >>>>> snd_pcm_close() will internally call  pcm->ops->close() which will fall to
> >>>>> snd_pcm_dmix_close() in case you are using dmix.
> >>>>>
> >>>>> snd_pcm_dmix_close() will try to acquire the semaphore with
> >>>>> snd_pcm_direct_semaphore_down(dmix, DIRECT_IPC_SEM_CLIENT).
> >>>>> The same semaphore is acquired in snd_pcm_dmix_sync_area() with
> >>>>> dmix_down_sem() in case of non-concurrent access.
> >>>>>
> >>>>> If semaphore is acquired in snd_pcm_dmix_sync_area() which is in thread
> >>>>> context and interrupt comes, which invokes the signal handler which is in
> >>>>> ISR context, which calls snd_pcm_close() which in turn calls
> >>>>> snd_pcm_dmix_close() then we see a deadlock since semaphore is not released
> >>>>> from thread context and ISR is waiting indefinitely on the same semaphore.
> >>>>>
> >>>>> Please suggest a suitable solution for this.
> >>>>
> >>>> It seems that also other configurations (alsa-lib plugins) have trouble
> >>>> with the closing from the signal handler - I hit mutex issues with the
> >>>> PulseAudio plugin, too.
> >>>>
> >>>> The question is, how we can do a clean path in this case. Looking to the
> >>>> current alsa-lib code, I would suggest to add the snd_pcm_abort()
> >>>> function (may be called from the interrupt handler) to notify the
> >>>> library to not ignore -EINTR return codes from poll() and other i/o ops
> >>>> and pass it to the caller (application) to finish the normal close sequence.
> >>>>
> >>>> Opinions?
> >>>
> >>> Isn't it only about the direct plugins with the case where no coherent
> >>> memory is available?  If so, we may just avoid the deadlock by
> >>> checking some internal flag like the patch below (untested)?
> >>> It's no perfect but just some proof, of course.
> >>
> >> It does not seem like a clean solution. It's just a workaround. I see
> >> similar locking problems inside the PulseAudio plugin (client library)
> >> when it's closed from the interrupt handler.
> > 
> > OK, that needs a fix.  But, the fix is anyway done to the plugin code
> > itself, so the necessary change would be pretty similar, I guess.
> > 
> >> Also, looking to apps, the
> >> best way is to handle the graceful shutdown outside the interrupt
> >> handler and remove whole "extra" code from the interrupt handler like
> >> the file header fixups (arecord) from it.
> > 
> > Yeah, agreed.  I would suggest to rewrite the code at first.
> > 
> > But this won't "fix" the existing code doing that.  It's a bad code,
> > but resulting in a silent deadlock just due to snd_pcm_close() doesn't
> > sound good, either.
> 
> I would just call abort() in this dead-lock case like the PA library
> code does. In this way, the problem will be reported. It's better than a
> silent acceptance.

Hm, maybe it's more educational, too.

> >> The "atomic" notification, that alsa-lib should abort all i/o wait
> >> operations, is a straigh way to do it. Again, syscalls returns EINTR,
> >> so we can detect this case and return to the app immediatelly.
> > 
> > The abort state handling itself looks useful.
> 
> Good.


Takashi