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

[Thomas Guillem]

I have an other use case:

 1/ Open a http stream, server is not responding, the open function of the http2 module is waiting
 2/ The user cancel if after some time, the http2 open module is interrupted
 3/ The old http module is probed and will also wait indefinitely. The user can't cancel it anymore.

For me, it's clearly a bug, and a possible fix should be backported to VLC 3.0 too.

I agree that my 2 attempts to fix it are not good

1/ Interrupt patch: rejected by you, I'll revert it
2/ module probe patch: rejected by you. And indeed, breaking module loading even when the user cancel a media is not a perfect solution. For example it could prevent to load a module that will just save some contexts.

I will work on an other solution

On Thu, Oct 24, 2019, at 11:59, Rémi Denis-Courmont wrote:
> 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é. 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20191024/a1c41cb9/attachment.html>