Welcome! Log In Create A New Profile

Advanced

[PATCH] BUG/MINOR: lua: Socket.send causing 'close' needs 1 arguments.

Posted by sada 
---
src/hlua.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/hlua.c b/src/hlua.c
index 60cf8f94..0585a1e7 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
/* The close function send shutdown signal and break the
* links between the stream and the object.
*/
-__LJMP static int hlua_socket_close(lua_State *L)
+__LJMP static int hlua_socket_close_helper(lua_State *L)
{
struct hlua_socket *socket;
struct appctx *appctx;
struct xref *peer;

- MAY_LJMP(check_args(L, 1, "close"));
-
socket = MAY_LJMP(hlua_checksocket(L, 1));

/* Check if we run on the same thread than the xreator thread.
@@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
return 0;
}

+/* The close function calls close_helper.
+ */
+__LJMP static int hlua_socket_close(lua_State *L)
+{
+ MAY_LJMP(check_args(L, 1, "close"));
+ return hlua_socket_close_helper(L);
+}
+
/* This Lua function assumes that the stack contain three parameters.
* 1 - USERDATA containing a struct socket
* 2 - INTEGER with values of the macro defined below
@@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
if (len == -1)
s->req.flags |= CF_WAKE_WRITE;

- MAY_LJMP(hlua_socket_close(L));
+ MAY_LJMP(hlua_socket_close_helper(L));
lua_pop(L, 1);
lua_pushinteger(L, -1);
xref_unlock(&socket->xref, peer);
--
2.17.0
Hi,
Did you get a chance to review the patch.

Thanks,
Sada.

On Fri, Apr 13, 2018 at 4:56 PM, sada <[email protected]> wrote:

> ---
> src/hlua.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/hlua.c b/src/hlua.c
> index 60cf8f94..0585a1e7 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
> /* The close function send shutdown signal and break the
> * links between the stream and the object.
> */
> -__LJMP static int hlua_socket_close(lua_State *L)
> +__LJMP static int hlua_socket_close_helper(lua_State *L)
> {
> struct hlua_socket *socket;
> struct appctx *appctx;
> struct xref *peer;
>
> - MAY_LJMP(check_args(L, 1, "close"));
> -
> socket = MAY_LJMP(hlua_checksocket(L, 1));
>
> /* Check if we run on the same thread than the xreator thread.
> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
> return 0;
> }
>
> +/* The close function calls close_helper.
> + */
> +__LJMP static int hlua_socket_close(lua_State *L)
> +{
> + MAY_LJMP(check_args(L, 1, "close"));
> + return hlua_socket_close_helper(L);
> +}
> +
> /* This Lua function assumes that the stack contain three parameters.
> * 1 - USERDATA containing a struct socket
> * 2 - INTEGER with values of the macro defined below
> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State
> *L,int status, lua_KContext
> if (len == -1)
> s->req.flags |= CF_WAKE_WRITE;
>
> - MAY_LJMP(hlua_socket_close(L));
> + MAY_LJMP(hlua_socket_close_helper(L));
> lua_pop(L, 1);
> lua_pushinteger(L, -1);
> xref_unlock(&socket->xref, peer);
> --
> 2.17.0
>
>
Hi. I miss this email, I will chek it ASAP.
Thierry

> On 19 Apr 2018, at 19:53, Sadasiva Gujarlapudi <[email protected]> wrote:
>
> Hi,
> Did you get a chance to review the patch.
>
> Thanks,
> Sada.
>
> On Fri, Apr 13, 2018 at 4:56 PM, sada <[email protected]> wrote:
> ---
> src/hlua.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/hlua.c b/src/hlua.c
> index 60cf8f94..0585a1e7 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
> /* The close function send shutdown signal and break the
> * links between the stream and the object.
> */
> -__LJMP static int hlua_socket_close(lua_State *L)
> +__LJMP static int hlua_socket_close_helper(lua_State *L)
> {
> struct hlua_socket *socket;
> struct appctx *appctx;
> struct xref *peer;
>
> - MAY_LJMP(check_args(L, 1, "close"));
> -
> socket = MAY_LJMP(hlua_checksocket(L, 1));
>
> /* Check if we run on the same thread than the xreator thread.
> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
> return 0;
> }
>
> +/* The close function calls close_helper.
> + */
> +__LJMP static int hlua_socket_close(lua_State *L)
> +{
> + MAY_LJMP(check_args(L, 1, "close"));
> + return hlua_socket_close_helper(L);
> +}
> +
> /* This Lua function assumes that the stack contain three parameters.
> * 1 - USERDATA containing a struct socket
> * 2 - INTEGER with values of the macro defined below
> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
> if (len == -1)
> s->req.flags |= CF_WAKE_WRITE;
>
> - MAY_LJMP(hlua_socket_close(L));
> + MAY_LJMP(hlua_socket_close_helper(L));
> lua_pop(L, 1);
> lua_pushinteger(L, -1);
> xref_unlock(&socket->xref, peer);
> --
> 2.17.0
>
>
Hi,do you have a little bit of context. It is not easy to read a patch withoutthe associated explanation.Thanks,ThierryOn 14 Apr 2018, at 01:56, sada &lt;[email protected]&gt; wrote:---src/hlua.c | 14 ++++++++++----1 file changed, 10 insertions(+), 4 deletions(-)diff --git a/src/hlua.c b/src/hlua.cindex 60cf8f94..0585a1e7 100644--- a/src/hlua.c+++ b/src/[email protected]@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)/* The close function send shutdown signal and break the* links between the stream and the object.*/-__LJMP static int hlua_socket_close(lua_State *L)+__LJMP static int hlua_socket_close_helper(lua_State *L){ struct hlua_socket *socket; struct appctx *appctx; struct xref *peer;- MAY_LJMP(check_args(L, 1, "close"));- socket = MAY_LJMP(hlua_checksocket(L, 1)); /* Check if we run on the same thread than the xreator [email protected]@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L) return 0;}+/* The close function calls close_helper.+ */+__LJMP static int hlua_socket_close(lua_State *L)+{+ MAY_LJMP(check_args(L, 1, "close"));+ return hlua_socket_close_helper(L);+}+/* This Lua function assumes that the stack contain three parameters.* &nbsp;1 - USERDATA containing a struct socket* &nbsp;2 - INTEGER with values of the macro defined [email protected]@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext if (len == -1) s-&gt;req.flags |= CF_WAKE_WRITE;- MAY_LJMP(hlua_socket_close(L));+ MAY_LJMP(hlua_socket_close_helper(L)); lua_pop(L, 1); lua_pushinteger(L, -1); xref_unlock(&amp;socket-&gt;xref, peer);-- 2.17.0
cc: Haproxy

`hlua_socket_close` expected exactly one argument on stack.
But when it was called from `hlua_socket_write_yield`, it found more than
one argument on stack and threw an error.

This patch introduced new helper function `hlua_socket_close_helper`, which
doesn't check arguments count.
Please let me know if you need more information.

Thanks,
Sada.


On Wed, Apr 25, 2018 at 10:51 AM, Thierry Fournier <[email protected]>
wrote:

> Hi,
>
> do you have a little bit of context. It is not easy to read a patch without
> the associated explanation.
>
> Thanks,
> Thierry
>
>
>
> On 14 Apr 2018, at 01:56, sada <[email protected]> wrote:
>
> ---
> src/hlua.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/hlua.c b/src/hlua.c
> index 60cf8f94..0585a1e7 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
> /* The close function send shutdown signal and break the
> * links between the stream and the object.
> */
> -__LJMP static int hlua_socket_close(lua_State *L)
> +__LJMP static int hlua_socket_close_helper(lua_State *L)
> {
> struct hlua_socket *socket;
> struct appctx *appctx;
> struct xref *peer;
>
> - MAY_LJMP(check_args(L, 1, "close"));
> -
> socket = MAY_LJMP(hlua_checksocket(L, 1));
>
> /* Check if we run on the same thread than the xreator thread.
> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
> return 0;
> }
>
> +/* The close function calls close_helper.
> + */
> +__LJMP static int hlua_socket_close(lua_State *L)
> +{
> + MAY_LJMP(check_args(L, 1, "close"));
> + return hlua_socket_close_helper(L);
> +}
> +
> /* This Lua function assumes that the stack contain three parameters.
> * 1 - USERDATA containing a struct socket
> * 2 - INTEGER with values of the macro defined below
> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State
> *L,int status, lua_KContext
> if (len == -1)
> s->req.flags |= CF_WAKE_WRITE;
>
> - MAY_LJMP(hlua_socket_close(L));
> + MAY_LJMP(hlua_socket_close_helper(L));
> lua_pop(L, 1);
> lua_pushinteger(L, -1);
> xref_unlock(&socket->xref, peer);
> --
> 2.17.0
>
>
>
This function checks that one argument are present in the stack, at the
bottom of the stack (position 1).

MAY_LJMP(check_args(L, 1, "close"));

The stack is kept between calls, so the position 1 will contains anything.
The content of the position 1 in the stack is a struct associated with the
socket object.

The function close is called byt the function xxx_write() which is a callback
of the Lua function “send()”. This send function have the same same first
argument then the Lua function "close()”, so the stack contains a socket object
at position 1.

I’m sure because the following line, just after the stack check gets
this argument:

socket = MAY_LJMP(hlua_checksocket(L, 1));

If your patch works, this is a proof that the stack contains expected value
at position 1, so removing the stack check doesn’t fix anything.

Maybe I miss something, or you encounters a bug caused by anything else.
Do you have a method for reproducing a bug ?

BR,
Thierry



> On 3 May 2018, at 08:56, Thierry Fournier <[email protected]> wrote:
>
> Hi,
>
> I’m not sure about your patch because the values in the stack are kept.
> I need to be focus to read it, and unfortunately I’m very busy.
> I try to check this morning, during the french strikes...
>
> Thierry
>
>
>> On 25 Apr 2018, at 20:26, Sadasiva Gujjarlapudi <[email protected]> wrote:
>>
>> cc: Haproxy
>>
>> `hlua_socket_close` expected exactly one argument on stack.
>> But when it was called from `hlua_socket_write_yield`, it found more than one argument on stack and threw an error.
>>
>> This patch introduced new helper function `hlua_socket_close_helper`, which doesn't check arguments count.
>> Please let me know if you need more information.
>>
>> Thanks,
>> Sada.
>>
>>
>> On Wed, Apr 25, 2018 at 10:51 AM, Thierry Fournier <[email protected]> wrote:
>> Hi,
>>
>> do you have a little bit of context. It is not easy to read a patch without
>> the associated explanation.
>>
>> Thanks,
>> Thierry
>>
>>
>>
>>> On 14 Apr 2018, at 01:56, sada <[email protected]> wrote:
>>>
>>> ---
>>> src/hlua.c | 14 ++++++++++----
>>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/hlua.c b/src/hlua.c
>>> index 60cf8f94..0585a1e7 100644
>>> --- a/src/hlua.c
>>> +++ b/src/hlua.c
>>> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
>>> /* The close function send shutdown signal and break the
>>> * links between the stream and the object.
>>> */
>>> -__LJMP static int hlua_socket_close(lua_State *L)
>>> +__LJMP static int hlua_socket_close_helper(lua_State *L)
>>> {
>>> struct hlua_socket *socket;
>>> struct appctx *appctx;
>>> struct xref *peer;
>>>
>>> - MAY_LJMP(check_args(L, 1, "close"));
>>> -
>>> socket = MAY_LJMP(hlua_checksocket(L, 1));
>>>
>>> /* Check if we run on the same thread than the xreator thread.
>>> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
>>> return 0;
>>> }
>>>
>>> +/* The close function calls close_helper.
>>> + */
>>> +__LJMP static int hlua_socket_close(lua_State *L)
>>> +{
>>> + MAY_LJMP(check_args(L, 1, "close"));
>>> + return hlua_socket_close_helper(L);
>>> +}
>>> +
>>> /* This Lua function assumes that the stack contain three parameters.
>>> * 1 - USERDATA containing a struct socket
>>> * 2 - INTEGER with values of the macro defined below
>>> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
>>> if (len == -1)
>>> s->req.flags |= CF_WAKE_WRITE;
>>>
>>> - MAY_LJMP(hlua_socket_close(L));
>>> + MAY_LJMP(hlua_socket_close_helper(L));
>>> lua_pop(L, 1);
>>> lua_pushinteger(L, -1);
>>> xref_unlock(&socket->xref, peer);
>>> --
>>> 2.17.0
>>>
>>
>>
>
This function checks that one argument are present in the stack, at the
bottom of the stack (position 1).

MAY_LJMP(check_args(L, 1, "close"));

The stack is kept between calls, so the position 1 will contains anything.
The content of the position 1 in the stack is a struct associated with the
socket object.

The function close is called byt the function xxx_write() which is a callback
of the Lua function “send()”. This send function have the same same first
argument then the Lua function "close()”, so the stack contains a socket object
at position 1.

I’m sure because the following line, just after the stack check gets
this argument:

socket = MAY_LJMP(hlua_checksocket(L, 1));

If your patch works, this is a proof that the stack contains expected value
at position 1, so removing the stack check doesn’t fix anything.

Maybe I miss something, or you encounters a bug caused by anything else.
Do you have a method for reproducing a bug ?

BR,
Thierry



> On 3 May 2018, at 08:56, Thierry Fournier <[email protected]> wrote:
>
> Hi,
>
> I’m not sure about your patch because the values in the stack are kept.
> I need to be focus to read it, and unfortunately I’m very busy.
> I try to check this morning, during the french strikes...
>
> Thierry
>
>
>> On 25 Apr 2018, at 20:26, Sadasiva Gujjarlapudi <[email protected]> wrote:
>>
>> cc: Haproxy
>>
>> `hlua_socket_close` expected exactly one argument on stack.
>> But when it was called from `hlua_socket_write_yield`, it found more than one argument on stack and threw an error.
>>
>> This patch introduced new helper function `hlua_socket_close_helper`, which doesn't check arguments count.
>> Please let me know if you need more information.
>>
>> Thanks,
>> Sada.
>>
>>
>> On Wed, Apr 25, 2018 at 10:51 AM, Thierry Fournier <[email protected]> wrote:
>> Hi,
>>
>> do you have a little bit of context. It is not easy to read a patch without
>> the associated explanation.
>>
>> Thanks,
>> Thierry
>>
>>
>>
>>> On 14 Apr 2018, at 01:56, sada <[email protected]> wrote:
>>>
>>> ---
>>> src/hlua.c | 14 ++++++++++----
>>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/hlua.c b/src/hlua.c
>>> index 60cf8f94..0585a1e7 100644
>>> --- a/src/hlua.c
>>> +++ b/src/hlua.c
>>> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
>>> /* The close function send shutdown signal and break the
>>> * links between the stream and the object.
>>> */
>>> -__LJMP static int hlua_socket_close(lua_State *L)
>>> +__LJMP static int hlua_socket_close_helper(lua_State *L)
>>> {
>>> struct hlua_socket *socket;
>>> struct appctx *appctx;
>>> struct xref *peer;
>>>
>>> - MAY_LJMP(check_args(L, 1, "close"));
>>> -
>>> socket = MAY_LJMP(hlua_checksocket(L, 1));
>>>
>>> /* Check if we run on the same thread than the xreator thread.
>>> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
>>> return 0;
>>> }
>>>
>>> +/* The close function calls close_helper.
>>> + */
>>> +__LJMP static int hlua_socket_close(lua_State *L)
>>> +{
>>> + MAY_LJMP(check_args(L, 1, "close"));
>>> + return hlua_socket_close_helper(L);
>>> +}
>>> +
>>> /* This Lua function assumes that the stack contain three parameters.
>>> * 1 - USERDATA containing a struct socket
>>> * 2 - INTEGER with values of the macro defined below
>>> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
>>> if (len == -1)
>>> s->req.flags |= CF_WAKE_WRITE;
>>>
>>> - MAY_LJMP(hlua_socket_close(L));
>>> + MAY_LJMP(hlua_socket_close_helper(L));
>>> lua_pop(L, 1);
>>> lua_pushinteger(L, -1);
>>> xref_unlock(&socket->xref, peer);
>>> --
>>> 2.17.0
>>>
>>
>>
>
Hi,

I’m not sure about your patch because the values in the stack are kept.
I need to be focus to read it, and unfortunately I’m very busy.
I try to check this morning, during the french strikes...

Thierry


> On 25 Apr 2018, at 20:26, Sadasiva Gujjarlapudi <[email protected]> wrote:
>
> cc: Haproxy
>
> `hlua_socket_close` expected exactly one argument on stack.
> But when it was called from `hlua_socket_write_yield`, it found more than one argument on stack and threw an error.
>
> This patch introduced new helper function `hlua_socket_close_helper`, which doesn't check arguments count.
> Please let me know if you need more information.
>
> Thanks,
> Sada.
>
>
> On Wed, Apr 25, 2018 at 10:51 AM, Thierry Fournier <[email protected]> wrote:
> Hi,
>
> do you have a little bit of context. It is not easy to read a patch without
> the associated explanation.
>
> Thanks,
> Thierry
>
>
>
>> On 14 Apr 2018, at 01:56, sada <[email protected]> wrote:
>>
>> ---
>> src/hlua.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/hlua.c b/src/hlua.c
>> index 60cf8f94..0585a1e7 100644
>> --- a/src/hlua.c
>> +++ b/src/hlua.c
>> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
>> /* The close function send shutdown signal and break the
>> * links between the stream and the object.
>> */
>> -__LJMP static int hlua_socket_close(lua_State *L)
>> +__LJMP static int hlua_socket_close_helper(lua_State *L)
>> {
>> struct hlua_socket *socket;
>> struct appctx *appctx;
>> struct xref *peer;
>>
>> - MAY_LJMP(check_args(L, 1, "close"));
>> -
>> socket = MAY_LJMP(hlua_checksocket(L, 1));
>>
>> /* Check if we run on the same thread than the xreator thread.
>> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
>> return 0;
>> }
>>
>> +/* The close function calls close_helper.
>> + */
>> +__LJMP static int hlua_socket_close(lua_State *L)
>> +{
>> + MAY_LJMP(check_args(L, 1, "close"));
>> + return hlua_socket_close_helper(L);
>> +}
>> +
>> /* This Lua function assumes that the stack contain three parameters.
>> * 1 - USERDATA containing a struct socket
>> * 2 - INTEGER with values of the macro defined below
>> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
>> if (len == -1)
>> s->req.flags |= CF_WAKE_WRITE;
>>
>> - MAY_LJMP(hlua_socket_close(L));
>> + MAY_LJMP(hlua_socket_close_helper(L));
>> lua_pop(L, 1);
>> lua_pushinteger(L, -1);
>> xref_unlock(&socket->xref, peer);
>> --
>> 2.17.0
>>
>
>
`check_args` checked for the "exact number of" arguments available on the
Lua stack as
given in the second param(nb).

So `close` was expecting exactly "one" argument on the Lua stack.
But when `close` was called from xxx_write_yield(), Lua stack had 3
arguments.

So close threw the exception with message "'close' needs 1 arguments".

"close_helper" function removed the Lua stack argument count check and only
checked
if the first argument was a socket.

----
void check_args(lua_State *L, int nb, char *fcn)
{
if (lua_gettop(L) == nb)
return;
WILL_LJMP(luaL_error(L, "'%s' needs %d arguments", fcn, nb));
}
----

Please let me know if you need more information.
Thanks,
Sada.


On Thu, May 3, 2018 at 12:16 AM, Thierry Fournier <[email protected]>
wrote:

> This function checks that one argument are present in the stack, at the
> bottom of the stack (position 1).
>
> MAY_LJMP(check_args(L, 1, "close"));
>
> The stack is kept between calls, so the position 1 will contains anything..
> The content of the position 1 in the stack is a struct associated with the
> socket object.
>
> The function close is called byt the function xxx_write() which is a
> callback
> of the Lua function “send()”. This send function have the same same first
> argument then the Lua function "close()”, so the stack contains a socket
> object
> at position 1.
>
> I’m sure because the following line, just after the stack check gets
> this argument:
>
> socket = MAY_LJMP(hlua_checksocket(L, 1));
>
> If your patch works, this is a proof that the stack contains expected value
> at position 1, so removing the stack check doesn’t fix anything.
>
> Maybe I miss something, or you encounters a bug caused by anything else.
> Do you have a method for reproducing a bug ?
>
> BR,
> Thierry
>
>
>
> > On 3 May 2018, at 08:56, Thierry Fournier <[email protected]>
> wrote:
> >
> > Hi,
> >
> > I’m not sure about your patch because the values in the stack are kept.
> > I need to be focus to read it, and unfortunately I’m very busy.
> > I try to check this morning, during the french strikes...
> >
> > Thierry
> >
> >
> >> On 25 Apr 2018, at 20:26, Sadasiva Gujjarlapudi <
> [email protected]> wrote:
> >>
> >> cc: Haproxy
> >>
> >> `hlua_socket_close` expected exactly one argument on stack.
> >> But when it was called from `hlua_socket_write_yield`, it found more
> than one argument on stack and threw an error.
> >>
> >> This patch introduced new helper function `hlua_socket_close_helper`,
> which doesn't check arguments count.
> >> Please let me know if you need more information.
> >>
> >> Thanks,
> >> Sada.
> >>
> >>
> >> On Wed, Apr 25, 2018 at 10:51 AM, Thierry Fournier <
> [email protected]> wrote:
> >> Hi,
> >>
> >> do you have a little bit of context. It is not easy to read a patch
> without
> >> the associated explanation.
> >>
> >> Thanks,
> >> Thierry
> >>
> >>
> >>
> >>> On 14 Apr 2018, at 01:56, sada <[email protected]> wrote:
> >>>
> >>> ---
> >>> src/hlua.c | 14 ++++++++++----
> >>> 1 file changed, 10 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/src/hlua.c b/src/hlua.c
> >>> index 60cf8f94..0585a1e7 100644
> >>> --- a/src/hlua.c
> >>> +++ b/src/hlua.c
> >>> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
> >>> /* The close function send shutdown signal and break the
> >>> * links between the stream and the object.
> >>> */
> >>> -__LJMP static int hlua_socket_close(lua_State *L)
> >>> +__LJMP static int hlua_socket_close_helper(lua_State *L)
> >>> {
> >>> struct hlua_socket *socket;
> >>> struct appctx *appctx;
> >>> struct xref *peer;
> >>>
> >>> - MAY_LJMP(check_args(L, 1, "close"));
> >>> -
> >>> socket = MAY_LJMP(hlua_checksocket(L, 1));
> >>>
> >>> /* Check if we run on the same thread than the xreator thread.
> >>> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State
> *L)
> >>> return 0;
> >>> }
> >>>
> >>> +/* The close function calls close_helper.
> >>> + */
> >>> +__LJMP static int hlua_socket_close(lua_State *L)
> >>> +{
> >>> + MAY_LJMP(check_args(L, 1, "close"));
> >>> + return hlua_socket_close_helper(L);
> >>> +}
> >>> +
> >>> /* This Lua function assumes that the stack contain three parameters.
> >>> * 1 - USERDATA containing a struct socket
> >>> * 2 - INTEGER with values of the macro defined below
> >>> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct
> lua_State *L,int status, lua_KContext
> >>> if (len == -1)
> >>> s->req.flags |= CF_WAKE_WRITE;
> >>>
> >>> - MAY_LJMP(hlua_socket_close(L));
> >>> + MAY_LJMP(hlua_socket_close_helper(L));
> >>> lua_pop(L, 1);
> >>> lua_pushinteger(L, -1);
> >>> xref_unlock(&socket->xref, peer);
> >>> --
> >>> 2.17.0
> >>>
> >>
> >>
> >
>
>
> On 3 May 2018, at 19:54, Sadasiva Gujjarlapudi <[email protected]> wrote:
>
> `check_args` checked for the "exact number of" arguments available on the Lua stack as
> given in the second param(nb).


You’re right ! Good catch.

Can you provide the patch with this lot of explanation for the commit ?

Like this: http://git.haproxy.org/?p=haproxy-1.8.git;a=commitdiff;h=05657bd24ebaf20e5c508a435be9a0830591f033

Thanks,
Thierry


> So `close` was expecting exactly "one" argument on the Lua stack.
> But when `close` was called from xxx_write_yield(), Lua stack had 3 arguments.
>
> So close threw the exception with message "'close' needs 1 arguments".
>
> "close_helper" function removed the Lua stack argument count check and only checked
> if the first argument was a socket.
>
> ----
> void check_args(lua_State *L, int nb, char *fcn)
> {
> if (lua_gettop(L) == nb)
> return;
> WILL_LJMP(luaL_error(L, "'%s' needs %d arguments", fcn, nb));
> }
> ----
>
> Please let me know if you need more information.
> Thanks,
> Sada.
>
>
> On Thu, May 3, 2018 at 12:16 AM, Thierry Fournier <[email protected]> wrote:
> This function checks that one argument are present in the stack, at the
> bottom of the stack (position 1).
>
> MAY_LJMP(check_args(L, 1, "close"));
>
> The stack is kept between calls, so the position 1 will contains anything.
> The content of the position 1 in the stack is a struct associated with the
> socket object.
>
> The function close is called byt the function xxx_write() which is a callback
> of the Lua function “send()”. This send function have the same same first
> argument then the Lua function "close()”, so the stack contains a socket object
> at position 1.
>
> I’m sure because the following line, just after the stack check gets
> this argument:
>
> socket = MAY_LJMP(hlua_checksocket(L, 1));
>
> If your patch works, this is a proof that the stack contains expected value
> at position 1, so removing the stack check doesn’t fix anything.
>
> Maybe I miss something, or you encounters a bug caused by anything else.
> Do you have a method for reproducing a bug ?
>
> BR,
> Thierry
>
>
>
> > On 3 May 2018, at 08:56, Thierry Fournier <[email protected]> wrote:
> >
> > Hi,
> >
> > I’m not sure about your patch because the values in the stack are kept.
> > I need to be focus to read it, and unfortunately I’m very busy.
> > I try to check this morning, during the french strikes...
> >
> > Thierry
> >
> >
> >> On 25 Apr 2018, at 20:26, Sadasiva Gujjarlapudi <[email protected]> wrote:
> >>
> >> cc: Haproxy
> >>
> >> `hlua_socket_close` expected exactly one argument on stack.
> >> But when it was called from `hlua_socket_write_yield`, it found more than one argument on stack and threw an error.
> >>
> >> This patch introduced new helper function `hlua_socket_close_helper`, which doesn't check arguments count.
> >> Please let me know if you need more information.
> >>
> >> Thanks,
> >> Sada.
> >>
> >>
> >> On Wed, Apr 25, 2018 at 10:51 AM, Thierry Fournier <[email protected]> wrote:
> >> Hi,
> >>
> >> do you have a little bit of context. It is not easy to read a patch without
> >> the associated explanation.
> >>
> >> Thanks,
> >> Thierry
> >>
> >>
> >>
> >>> On 14 Apr 2018, at 01:56, sada <[email protected]> wrote:
> >>>
> >>> ---
> >>> src/hlua.c | 14 ++++++++++----
> >>> 1 file changed, 10 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/src/hlua.c b/src/hlua.c
> >>> index 60cf8f94..0585a1e7 100644
> >>> --- a/src/hlua.c
> >>> +++ b/src/hlua.c
> >>> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
> >>> /* The close function send shutdown signal and break the
> >>> * links between the stream and the object.
> >>> */
> >>> -__LJMP static int hlua_socket_close(lua_State *L)
> >>> +__LJMP static int hlua_socket_close_helper(lua_State *L)
> >>> {
> >>> struct hlua_socket *socket;
> >>> struct appctx *appctx;
> >>> struct xref *peer;
> >>>
> >>> - MAY_LJMP(check_args(L, 1, "close"));
> >>> -
> >>> socket = MAY_LJMP(hlua_checksocket(L, 1));
> >>>
> >>> /* Check if we run on the same thread than the xreator thread.
> >>> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
> >>> return 0;
> >>> }
> >>>
> >>> +/* The close function calls close_helper.
> >>> + */
> >>> +__LJMP static int hlua_socket_close(lua_State *L)
> >>> +{
> >>> + MAY_LJMP(check_args(L, 1, "close"));
> >>> + return hlua_socket_close_helper(L);
> >>> +}
> >>> +
> >>> /* This Lua function assumes that the stack contain three parameters.
> >>> * 1 - USERDATA containing a struct socket
> >>> * 2 - INTEGER with values of the macro defined below
> >>> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
> >>> if (len == -1)
> >>> s->req.flags |= CF_WAKE_WRITE;
> >>>
> >>> - MAY_LJMP(hlua_socket_close(L));
> >>> + MAY_LJMP(hlua_socket_close_helper(L));
> >>> lua_pop(L, 1);
> >>> lua_pushinteger(L, -1);
> >>> xref_unlock(&socket->xref, peer);
> >>> --
> >>> 2.17.0
> >>>
> >>
> >>
> >
>
>
Function `hlua_socke_close` expected exactly one argument on the Lua stack.
But when `hlua_socket_close` was called from `hlua_socket_write_yield`,
Lua stack had 3 arguments. So `hlua_socket_close` threw the exception with
message "'close' needs 1 arguments".

Introduced new helper function `hlua_socket_close_helper`, which removed the
Lua stack argument count check and only checked if the first argument was
a socket.

This fix should be backported to 1.8, 1.7 and 1.6.
---
src/hlua.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/hlua.c b/src/hlua.c
index d07e8d67..8cc30513 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
/* The close function send shutdown signal and break the
* links between the stream and the object.
*/
-__LJMP static int hlua_socket_close(lua_State *L)
+__LJMP static int hlua_socket_close_helper(lua_State *L)
{
struct hlua_socket *socket;
struct appctx *appctx;
struct xref *peer;

- MAY_LJMP(check_args(L, 1, "close"));
-
socket = MAY_LJMP(hlua_checksocket(L, 1));

/* Check if we run on the same thread than the xreator thread.
@@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
return 0;
}

+/* The close function calls close_helper.
+ */
+__LJMP static int hlua_socket_close(lua_State *L)
+{
+ MAY_LJMP(check_args(L, 1, "close"));
+ return hlua_socket_close_helper(L);
+}
+
/* This Lua function assumes that the stack contain three parameters.
* 1 - USERDATA containing a struct socket
* 2 - INTEGER with values of the macro defined below
@@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
if (len == -1)
s->req.flags |= CF_WAKE_WRITE;

- MAY_LJMP(hlua_socket_close(L));
+ MAY_LJMP(hlua_socket_close_helper(L));
lua_pop(L, 1);
lua_pushinteger(L, -1);
xref_unlock(&socket->xref, peer);
--
2.17.0
Hi Sada,

Thanks for the patch,
Willy, can you apply ?

BR,
Thierry


On Fri, 11 May 2018 11:48:18 -0700
sada <[email protected]> wrote:

> Function `hlua_socke_close` expected exactly one argument on the Lua stack.
> But when `hlua_socket_close` was called from `hlua_socket_write_yield`,
> Lua stack had 3 arguments. So `hlua_socket_close` threw the exception with
> message "'close' needs 1 arguments".
>
> Introduced new helper function `hlua_socket_close_helper`, which removed the
> Lua stack argument count check and only checked if the first argument was
> a socket.
>
> This fix should be backported to 1.8, 1.7 and 1.6.
> ---
> src/hlua.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/src/hlua.c b/src/hlua.c
> index d07e8d67..8cc30513 100644
> --- a/src/hlua.c
> +++ b/src/hlua.c
> @@ -1629,14 +1629,12 @@ __LJMP static int hlua_socket_gc(lua_State *L)
> /* The close function send shutdown signal and break the
> * links between the stream and the object.
> */
> -__LJMP static int hlua_socket_close(lua_State *L)
> +__LJMP static int hlua_socket_close_helper(lua_State *L)
> {
> struct hlua_socket *socket;
> struct appctx *appctx;
> struct xref *peer;
>
> - MAY_LJMP(check_args(L, 1, "close"));
> -
> socket = MAY_LJMP(hlua_checksocket(L, 1));
>
> /* Check if we run on the same thread than the xreator thread.
> @@ -1659,6 +1657,14 @@ __LJMP static int hlua_socket_close(lua_State *L)
> return 0;
> }
>
> +/* The close function calls close_helper.
> + */
> +__LJMP static int hlua_socket_close(lua_State *L)
> +{
> + MAY_LJMP(check_args(L, 1, "close"));
> + return hlua_socket_close_helper(L);
> +}
> +
> /* This Lua function assumes that the stack contain three parameters.
> * 1 - USERDATA containing a struct socket
> * 2 - INTEGER with values of the macro defined below
> @@ -1990,7 +1996,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
> if (len == -1)
> s->req.flags |= CF_WAKE_WRITE;
>
> - MAY_LJMP(hlua_socket_close(L));
> + MAY_LJMP(hlua_socket_close_helper(L));
> lua_pop(L, 1);
> lua_pushinteger(L, -1);
> xref_unlock(&socket->xref, peer);
> --
> 2.17.0
>
>
On Fri, May 18, 2018 at 12:46:07PM +0200, thierry.fournier@arpalert.org wrote:
> Hi Sada,
>
> Thanks for the patch,
> Willy, can you apply ?

Now done, thanks!

Willy
Sorry, only registered users may post in this forum.

Click here to login