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

[Thomas Guillem]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20191024/6f551692/attachment.html>