On Sat, 26 May 2018 19:47:54 +0200
PiBa-NL <piba.nl....@gmail.com> wrote:
> Hi Thierry,
>
> Op 25-5-2018 om 15:40 schreef Thierry FOURNIER:
> > On Fri, 18 May 2018 22:17:00 +0200
> > PiBa-NL <piba.nl....@gmail.com> 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
>
> Thanks, seems both the hang and the 2nd uncovered task schedule issue
> are fixed now (google website response is received/processed fast
> again). I've done some testing, and installed the updated/patched
> version on my production box last night. At the moment it still works
> properly. Activated my lua healthchecker and mailer tasks and enabled 3
> threads again.. Lets see how it go's :), but as said for now it seems to
> work alright.
>
> Does the second issue you found and fixed clear the initial 'doubts
> about the reliability' of the first one.? Or did you have a particular
> possibly problematic scenario in mind that i could try and check for?
>
> For the moment i think it is more 'reliable / stable' with the patches
> than without. So in that regard i think they could be merged.
>
> There seems to be a issue with tcp:settimeout() though. But ill put that
> in a different mail-threat as im not sure its related and the issues
> where this thread started are fixed.
I tried a case never encountered before. When the data sent is bigger than
an haproxy buffer, the send function is absolutely bugged, so I submit
some additional patches.
BR,
Thierry
> Regards,
>
> PiBa-NL (Pieter)
>
>From 4f8fb1ea81e9e33005f52754775b6dada035bf22 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <thierry.fourn...@ozon.io>
Date: Fri, 25 May 2018 14:38:57 +0200
Subject: [PATCH 1/5] 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 4de5db5ac..03e961c37 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1799,7 +1799,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"));
@@ -1951,7 +1950,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;
}
@@ -1959,7 +1957,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 cce5f285c78c3bacd24212ccfca45a460dc50090 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <thierry.fourn...@ozon.io>
Date: Fri, 25 May 2018 15:03:50 +0200
Subject: [PATCH 2/5] 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 | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/hlua.c b/src/hlua.c
index 03e961c37..48de5cba5 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1766,8 +1766,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.
--
2.16.3
>From 5583558c97b8a07dba40f655fbb27ed4320ecbd4 Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <thierry.fourn...@ozon.io>
Date: Sun, 27 May 2018 00:59:48 +0200
Subject: [PATCH 3/5] BUG/MEDIUM: lua/socket: Notification error
Each time the send fucntion yields, a notification must be registered.
Without this notification, the task is never wakeup when data arrives.
---
src/hlua.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/hlua.c b/src/hlua.c
index 48de5cba5..5d49d3c03 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1956,10 +1956,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) {
- 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"));
- }
goto hlua_socket_write_yield_return;
}
@@ -2002,6 +1998,10 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
}
hlua_socket_write_yield_return:
+ 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"));
+ }
xref_unlock(&socket->xref, peer);
WILL_LJMP(hlua_yieldk(L, 0, 0, hlua_socket_write_yield, TICK_ETERNITY, 0));
return 0;
--
2.16.3
>From 9baf13f8e8158054a1053cecae1c68dbbc43eb3c Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <thierry.fourn...@ozon.io>
Date: Sun, 27 May 2018 01:27:40 +0200
Subject: [PATCH 4/5] BUG/MEDIUM: lua/socket: Applet attached to socket not
wakeup
stream_int_notify() cant be used because it wakeup the applet
task only if the stream have changes. The changes are forced
by Lua, but not repported on the stream, so we must force the
wakeup using appctx_wakeup();
---
src/hlua.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/hlua.c b/src/hlua.c
index 5d49d3c03..2f86ffc02 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1982,7 +1982,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
>From 9ac84db9d6ac929ca8d3a1fd9a8fc97e43d9df1c Mon Sep 17 00:00:00 2001
From: Thierry FOURNIER <thierry.fourn...@ozon.io>
Date: Sun, 27 May 2018 01:14:47 +0200
Subject: [PATCH 5/5] BUG/MEDIUM: lua/socket: Buffer error, may segfault
The buffer pointer is already updated. It is again updated
when it is given to the function ci_putblk().
---
src/hlua.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/hlua.c b/src/hlua.c
index 2f86ffc02..51c35e989 100644
--- a/src/hlua.c
+++ b/src/hlua.c
@@ -1962,7 +1962,7 @@ static int hlua_socket_write_yield(struct lua_State *L,int status, lua_KContext
/* send data */
if (len < send_len)
send_len = len;
- len = ci_putblk(&s->req, buf+sent, send_len);
+ len = ci_putblk(&s->req, buf, send_len);
/* "Not enough space" (-1), "Buffer too little to contain
* the data" (-2) are not expected because the available length
--
2.16.3
