Hi Cyril,

On Sun, Aug 12, 2018 at 10:49:13PM +0200, Cyril Bonté wrote:
> 2. hlua_socket_settimeout() initializes rto/wto values, maybe it should also
> compute the rex/wex values :
>       socket->s->req.rex = tick_add_ifset(now_ms, tmout);
>       socket->s->req.wex = tick_add_ifset(now_ms, tmout);
>       socket->s->res.rex = tick_add_ifset(now_ms, tmout);
>       socket->s->res.wex = tick_add_ifset(now_ms, tmout);

You're absolutely right, it's indeed incorrect to set {r,w}to without
updating the expire date. It only works when the timeout is extended,
but not when it's shortened.

> 3. It may require to wake up the task if a new timeout is set after a first
> one was already set (in your case the task doesn't wake up after 2 secondes
> because a first timeout was set to 3 seconds) :
>       task_wakeup(socket->s->task, TASK_WOKEN_OTHER);

Normally it should not be needed, however it needs to be requeued to
take care of the fact that the timeout might have been shortened :

        socket->s->task->expire = tick_add_ifset(now_ms, tmout);
        task_queue(socket->s->task);

> At least, it seems to fix the issue but before sending a patch, I want to be
> sure that's how we should fix this.

If you can just validate that the two lines above are enough, that would
be great. Avoiding to call process_stream() just to update a timeout is
desirable. If for any reason it doesn't work or seems more complicated,
then the wakeup as you proposed should indeed work.

Thanks,
Willy

Reply via email to