Re: sound: uninterruptible hang in snd_seq_oss_writeq_sync

[Dmitry Vyukov]

On Tue, Mar 1, 2016 at 4:31 PM, Takashi Iwai <tiwai@xxxxxxx> wrote:
> On Tue, 01 Mar 2016 16:04:43 +0100,
> Dmitry Vyukov wrote:
>>
>> On Tue, Mar 1, 2016 at 4:01 PM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
>> > On Tue, Mar 1, 2016 at 3:44 PM, Takashi Iwai <tiwai@xxxxxxx> wrote:
>> >> On Tue, 01 Mar 2016 13:33:27 +0100,
>> >> Dmitry Vyukov wrote:
>> >>>
>> >>> Hello,
>> >>>
>> >>> The following program creates an unkillable process:
>> >> .....
>> >>> The hang stack is:
>> >>>
>> >>> [<ffffffff85309f77>] snd_seq_oss_writeq_sync+0x327/0x790
>> >>> sound/core/seq/oss/seq_oss_writeq.c:121
>> >>
>> >> This is
>> >>         wait_event_interruptible_timeout(q->sync_sleep, ! q->sync_event_put, HZ);
>> >>
>> >> and this should return zero
>> >>         if (signal_pending(current))
>> >>                 /* interrupted - return 0 to finish sync */
>> >>                 q->sync_event_put = 0;
>> >>         if (! q->sync_event_put || q->sync_time >= time)
>> >>                 return 0;
>> >>         return 1;
>> >>
>> >>> [<ffffffff852fa353>] snd_seq_oss_drain_write+0x113/0x160
>> >>
>> >> ... and this loop should break:
>> >>                 while (snd_seq_oss_writeq_sync(dp->writeq))
>> >>                         ;
>> >>
>> >> So, I see no obvious error in the code, so far.
>> >>
>> >> I'm running your test program now with 8 parallel runs, but I couldn't
>> >> reproduce it.  Any other specifics?
>>
>>
>> Hummm.. for me this process hangs every time, even if I just run it
>> once from console.
>> I've now retested in on clean commit
>> fc77dbd34c5c99bce46d40a2491937c3bcbd10af (without KASAN/KCOV and with
>> release gcc) and also got the same hang.
>> I can only think of a different config. I've attached mine, please try with it.
>>
>> Are signals delivered if we are already in process of dying?
>> When I kill -9 this process it does not react, so presumably wait is
>> not unblocked...
>
> Good point.
>
> But above all, it doesn't make much sense to loop this sync call.
> When wait_event*() returns, it should go out.  So, the patch like
> below should be good enough and fix your issue, too.



The patch fixes the hang for me. There is a second delay at exit, but
otherwise the process exits fine.

Tested-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>

Thanks for quick fix!

> ---
> diff --git a/sound/core/seq/oss/seq_oss_writeq.c b/sound/core/seq/oss/seq_oss_writeq.c
> index 1f6788a18444..4014a5469d1a 100644
> --- a/sound/core/seq/oss/seq_oss_writeq.c
> +++ b/sound/core/seq/oss/seq_oss_writeq.c
> @@ -119,12 +119,7 @@ snd_seq_oss_writeq_sync(struct seq_oss_writeq *q)
>         }
>
>         wait_event_interruptible_timeout(q->sync_sleep, ! q->sync_event_put, HZ);
> -       if (signal_pending(current))
> -               /* interrupted - return 0 to finish sync */
> -               q->sync_event_put = 0;
> -       if (! q->sync_event_put || q->sync_time >= time)
> -               return 0;
> -       return 1;
> +       return 0;
>  }
>
>  /*