On Fri, 18 May 2018 22:17:00 +0200
PiBa-NL <[email protected]> wrote:
> Hi Thierry,
>
> Op 18-5-2018 om 20:00 schreef Thierry FOURNIER:
> > Hi Pieter,
> >
> > Could you test the attached patch ? It seems to fix the problem, but I
> > have some doubts about the reliability of the patch.
> >
> > Thierry
> The crash seems 'fixed' indeed.. but the lua scipt below now takes
> 5seconds instead of 150ms.
>
> Regards,
> PiBa-NL (Pieter)
>
> con = core.tcp()
> con:connect_ssl("216.58.212.132",443) --google: 216.58.212.132
> request = [[GET / HTTP/1.0
>
> ]]
> con:send(request)
> res = con:receive("*a")
> con:close()
One bug can hide another bug :-) I catch both. Could you test ?
If the result is positive I join also the backport for 1.6 and 1.7
Thierry
>From 498b9d4e76b599e533ff31d0d3c14dd1726d00f5 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <[email protected]>
Date: Fri, 25 May 2018 14:38:57 +0200
Subject: [PATCH 1/2] BUG/MEDIUM: lua/socket: wrong scheduling for sockets
The appctx pointer inherit from pre-thread dev. The thread gives new
link system called xref which maintain a link between the socket ignitor
and the applet. appctx is given at once at the start fo the function.
Inheritage import wrong method for getting the appctx. This implies
the wakeup of wrong applet, and the socket are no longer responsive.
This behavior is hidden by another inherited error whic is fixed in the
next patch.
This patch remove all wrong appctx affectations.
This patch must be backported in 1.6, 1.7 and 1.8
---
src/hlua.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/hlua.c b/src/hlua.c
index a476bcc3d..f8d3836ef 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1689,6 +1689,8 @@ __LJMP static int hlua_socket_receive_yield(struct lua_State *L, int status, lua
if (!socket->s)
goto connection_closed;
+ appctx = objt_appctx(socket->s->si[0].end);
+
oc = &socket->s->res;
if (wanted == HLSR_READ_LINE) {
/* Read line. */
@@ -1785,7 +1787,6 @@ connection_closed:
connection_empty:
- appctx = objt_appctx(socket->s->si[0].end);
if (!hlua_com_new(hlua, &appctx->ctx.hlua.wake_on_read))
WILL_LJMP(luaL_error(L, "out of memory"));
WILL_LJMP(hlua_yieldk(L, 0, 0, hlua_socket_receive_yield, TICK_ETERNITY, 0));
@@ -1894,6 +1895,8 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
return 1;
}
+ appctx = objt_appctx(socket->s->si[0].end);
+
/* Update the input buffer data. */
buf += sent;
send_len = buf_len - sent;
@@ -1906,7 +1909,6 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
* the request buffer if its not required.
*/
if (socket->s->req.buf->size == 0) {
- appctx = hlua->task->context;
if (!channel_alloc_buffer(&socket->s->req, &appctx->buffer_wait))
goto hlua_socket_write_yield_return;
}
@@ -1914,7 +1916,6 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
/* Check for avalaible space. */
len = buffer_total_space(socket->s->req.buf);
if (len <= 0) {
- appctx = objt_appctx(socket->s->si[0].end);
if (!hlua_com_new(hlua, &appctx->ctx.hlua.wake_on_write))
WILL_LJMP(luaL_error(L, "out of memory"));
goto hlua_socket_write_yield_return;
--
2.16.3
>From d61022902ff1489f9343a3f430ad1b2b9efd23c7 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <[email protected]>
Date: Fri, 25 May 2018 15:03:50 +0200
Subject: [PATCH 2/2] BUG/MAJOR: lua: Dead lock with sockets
In some cases, when we are waiting for data and the socket
timeout expires, we have a dead lock. The Lua socket locks
the applet socket, and call for a notify. The notify
immediately executes code and try to acquire the same lock,
so ... dead lock.
stream_int_notify() cant be used because it wakeup the applet
task only if the stream have changes. The changes are forces
by Lua, but not repported on the stream.
stream_int_update_applet() cant be used because the deadlock.
So, I inconditionnaly wakeup the applet. This wake is performed
asynchronously, and will call a stream_int_notify().
This patch must be backported in 1.6, 1.7 and 1.8
---
src/hlua.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/hlua.c b/src/hlua.c
index f8d3836ef..de5afa6c5 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1759,8 +1759,7 @@ __LJMP static int hlua_socket_receive_yield(struct lua_State *L, int status, lua
bo_skip(oc, len + skip_at_end);
/* Don't wait anything. */
- stream_int_notify(&socket->s->si[0]);
- stream_int_update_applet(&socket->s->si[0]);
+ appctx_wakeup(appctx);
/* If the pattern reclaim to read all the data
* in the connection, got out.
@@ -1942,8 +1941,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
}
/* update buffers. */
- stream_int_notify(&socket->s->si[0]);
- stream_int_update_applet(&socket->s->si[0]);
+ appctx_wakeup(appctx);
socket->s->req.rex = TICK_ETERNITY;
socket->s->res.wex = TICK_ETERNITY;
--
2.16.3
>From d1c3f87edfea573d697e9ef2b3817c637925d36b Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <[email protected]>
Date: Fri, 25 May 2018 14:38:57 +0200
Subject: [PATCH 1/2] BUG/MEDIUM: lua/socket: wrong scheduling for sockets
The appctx pointer inherit from pre-thread dev. The thread gives new
link system called xref which maintain a link between the socket ignitor
and the applet. appctx is given at once at the start fo the function.
Inheritage import wrong method for getting the appctx. This implies
the wakeup of wrong applet, and the socket are no longer responsive.
This behavior is hidden by another inherited error whic is fixed in the
next patch.
This patch remove all wrong appctx affectations.
This patch must be backported in 1.6, 1.7 and 1.8
---
src/hlua.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/hlua.c b/src/hlua.c
index 727d66484..1d17c6cf2 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1797,7 +1797,6 @@ no_peer:
connection_empty:
- appctx = objt_appctx(s->si[0].end);
if (!notification_new(&hlua->com, &appctx->ctx.hlua_cosocket.wake_on_read, hlua->task)) {
xref_unlock(&socket->xref, peer);
WILL_LJMP(luaL_error(L, "out of memory"));
@@ -1949,7 +1948,6 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
* the request buffer if its not required.
*/
if (s->req.buf->size == 0) {
- appctx = hlua->task->context;
if (!channel_alloc_buffer(&s->req, &appctx->buffer_wait))
goto hlua_socket_write_yield_return;
}
@@ -1957,7 +1955,6 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
/* Check for avalaible space. */
len = buffer_total_space(s->req.buf);
if (len <= 0) {
- appctx = objt_appctx(s->si[0].end);
if (!notification_new(&hlua->com, &appctx->ctx.hlua_cosocket.wake_on_write, hlua->task)) {
xref_unlock(&socket->xref, peer);
WILL_LJMP(luaL_error(L, "out of memory"));
--
2.16.3
>From 948ce0f2d8bd11b83772f043af4f0cb2b26602a3 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <[email protected]>
Date: Fri, 25 May 2018 15:03:50 +0200
Subject: [PATCH 2/2] BUG/MAJOR: lua: Dead lock with sockets
In some cases, when we are waiting for data and the socket
timeout expires, we have a dead lock. The Lua socket locks
the applet socket, and call for a notify. The notify
immediately executes code and try to acquire the same lock,
so ... dead lock.
stream_int_notify() cant be used because it wakeup the applet
task only if the stream have changes. The changes are forces
by Lua, but not repported on the stream.
stream_int_update_applet() cant be used because the deadlock.
So, I inconditionnaly wakeup the applet. This wake is performed
asynchronously, and will call a stream_int_notify().
This patch must be backported in 1.6, 1.7 and 1.8
---
src/hlua.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/hlua.c b/src/hlua.c
index 1d17c6cf2..02e0beea4 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1764,8 +1764,7 @@ __LJMP static int hlua_socket_receive_yield(struct lua_State *L, int status, lua
co_skip(oc, len + skip_at_end);
/* Don't wait anything. */
- stream_int_notify(&s->si[0]);
- stream_int_update_applet(&s->si[0]);
+ appctx_wakeup(appctx);
/* If the pattern reclaim to read all the data
* in the connection, got out.
@@ -1984,8 +1983,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
}
/* update buffers. */
- stream_int_notify(&s->si[0]);
- stream_int_update_applet(&s->si[0]);
+ appctx_wakeup(appctx);
s->req.rex = TICK_ETERNITY;
s->res.wex = TICK_ETERNITY;
--
2.16.3
