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

[Rémi Denis-Courmont]

That's easy: drop the http alias from the old module (and rename it Icecast). This bug has nothing to do with user abort; we shouldn't retry to connect to an unresponsive IP address.

Le 24 octobre 2019 13:47:02 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>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

-- 
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/c6afdd42/attachment.html>