[vlc-devel] [PATCH 1/2] interrupt: restore interrupted to the killed state

[Rémi Denis-Courmont]

I don't agree with the problem statement. What it describes is intended behaviour, not a bug.

Le 24 octobre 2019 12:46:46 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>
>On Thu, Oct 24, 2019, at 11:20, Rémi Denis-Courmont wrote:
>> You effectively reverted to the old broken logic from before the
>interrupt system, and disregarding documented API semantics, wrecking
>many weeks of work from me. Is what you did.
>
>Yes I made a mistake, I should have waited longer and/or ask again for
>review before pushing it.
>I did not realize that my patch would cause such a mess. I'm sorry for
>that.
>
>Is my alternate solution acceptable for you ?
>
>> 
>> Le 24 octobre 2019 10:02:17 GMT+03:00, Thomas Guillem
><thomas at gllm.fr> a écrit :
>>> 
>>> 
>>> On Wed, Oct 23, 2019, at 21:25, Remi Denis-Courmont wrote:
>>>> Le 2019-10-22 17:47, Thomas Guillem a écrit :
>>>>> If a module is interrupted while waiting in a vlc_*_i11e*()
>function, 
>>>>> the i11e
>>>>> function will return an error after restoring the interrupted
>state 
>>>>> (from
>>>>> vlc_interrupt_finish()). This error will be carefully handled by
>the 
>>>>> module
>>>>> that will close itself. If a second module is in the probe list,
>any 
>>>>> call to a
>>>>> vlc_*_i11e*() function will act as not interrupted. This cause any
>
>>>>> following
>>>>> modules in the probe list to ignore this interrupted state.
>>>>> 
>>>>> To fix this issue, vlc_interrupt_finish() will now restore the 
>>>>> interrupted
>>>>> state to the killed state. src/misc/interrupt.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/src/misc/interrupt.c b/src/misc/interrupt.c
>>>>> index d2a04a72b3c..54810892fc1 100644
>>>>> --- a/src/misc/interrupt.c
>>>>> +++ b/src/misc/interrupt.c
>>>>> @@ -151,7 +151,7 @@ static int
>vlc_interrupt_finish(vlc_interrupt_t 
>>>>> *ctx)
>>>>>      if (ctx->interrupted)
>>>>>      {
>>>>>          ret = EINTR;
>>>>> -        ctx->interrupted = false;
>>>>> +        ctx->interrupted = atomic_load(&ctx->killed);
>>>> This looks wrong. The interruption is only there to signal the
>change in 
>>>> status. It's not supposed to fire again if the status is not
>changed.
>>> 
>>> I sent this patch to open a discussion, I pushed the next day, maybe
>way too early, because vlc for iOS really need it in order to fix
>dialogs that are shown after the media is killed.
>>> 
>>> It's OK I can revert.
>>> 
>>> But I would like to discuss about the other way to fix the issue
>mentioned by this patch.
>>> The only other fix I see if to abort the module probing (between 2
>probes) if vlc_killed().
>>> 
>>> > 
>>>>> }
>>>>> vlc_mutex_unlock(&ctx->lock);
>>>>> return ret;
>>>> -- 
>>>> Rémi Denis-Courmontvlc-devel mailing list
>>>> To unsubscribe or modify your subscription options:
>>>> https://mailman.videolan.org/listinfo/vlc-devel
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> 
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté. 
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20191024/e943e0f5/attachment.html>