I catch the bug. It affect all the versions of HAProxy embedding Lua. It is fixed.
I join 2 main patches: - 0001*.patch: the bug fix, it should be backported - 0002*.patch: a nice cleanup which is useless to backport And I join the backport for the 1.6 version because it is not trivial. - 0001-*.1.6.patch Willy, can you merge these bugfix ? Thierry On Wed, 26 Apr 2017 07:51:04 -0400 Philip Seidel <[email protected]> wrote: > Thierry -- > > Thanks for your help with this. I tried running with that line commented > out and I still see the same behavior during my testing. Memory is still > never reclaimed throughout the life of the haproxy process. > > Thanks, > > -Phil > > On Wed, Apr 26, 2017 at 7:09 AM, <[email protected]> wrote: > > > It seems that the memleak doesn't appear when if remove the line which > > sets the "priv" context do nil. > > > > So, when I comment this line: > > > > txn:set_priv(txnPrvState); > > > > The memleak disappear. Now if you want, you can apply this workaroud. > > I try to understand the memleak. > > > > Thierry > > > > > > On Wed, 26 Apr 2017 12:11:16 +0200 > > Thierry Fournier <[email protected]> wrote: > > > > > Hi, > > > > > > Thanks for the bug report. I can reproduce the bug. > > > I will look for a fix. > > > > > > Thierry > > > > > > > > > > On 26 Apr 2017, at 05:14, Philip Seidel <[email protected]> > > wrote: > > > > > > > > Possible Lua Memory Leak? > > > > > > > > We are running HAProxy version 1.7.2 (also tested 1.7.5) and are > > loading a Lua file which makes use of 2 actions. The first action is > > called on http-request and the second on http-response. In the simple test > > case we put together the request action stores some data in a table and > > calls txn:set_priv to save the state for the transaction. The response > > action calls txn:get_priv in order to access the state that was set in the > > request action. All variables are set to nil once they are no longer > > needed but it seems that no matter what kind of load HAProxy is receiving > > we leak a little bit of memory with each request. Eventually, response > > times begin to increase to the point where health checks to HAProxy begin > > to fail and the server is unresponsive. We can take the instance out of > > rotation and memory still doesn't get reclaimed despite all connections to > > the frontend and backend being closed. We do see garbage collections > > happening but it never cleans up enough to stabi > > lize the instance. The leak is easily reproducible using a local test > > instance and JMeter. The test uses 10 connections with keep-alive at > > around 1500 r/s. It takes only a few minutes to consume over 1.5G of > > memory. Any ideas as to what might be going on here? Is there something > > wrong with how we are attempting to integrate these LUA actions? I am > > happy to provide additional information if anyone is willing to assist with > > this. I have posted the configuration and other files on pastebin. > > > > > > > > Configuration - https://pastebin.com/SBtAEZ9R > > > > Lua - https://pastebin.com/64Anbxm4 > > > > > > > > > > > >
>From ea1dc9b8dcb0bcf340132402d23648a2c397a4b3 Mon Sep 17 00:00:00 2001 From: Thierry FOURNIER <[email protected]> Date: Wed, 26 Apr 2017 14:25:58 +0200 Subject: [PATCH] BUG/MEDIUM: lua: memory leak The priv context is not cleaned when we set a new priv context. This is caused by a stupid swap between two parameter of the luaL_unref() function. workaround: use set_priv only once when we process a stream. This patch should be backported in version 1.7 and 1.6 --- src/hlua.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/hlua.c b/src/hlua.c index 4e35785..d276e3a 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -3615,6 +3615,40 @@ static int hlua_applet_http_new(lua_State *L, struct appctx *ctx) return 1; } +<<<<<<< HEAD +======= +__LJMP static int hlua_applet_http_set_priv(lua_State *L) +{ + struct hlua_appctx *appctx = MAY_LJMP(hlua_checkapplet_http(L, 1)); + struct stream *s = appctx->htxn.s; + struct hlua *hlua = &s->hlua; + + MAY_LJMP(check_args(L, 2, "set_priv")); + + /* Remove previous value. */ + if (hlua->Mref != -1) + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); + + /* Get and store new value. */ + lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ + hlua->Mref = luaL_ref(L, LUA_REGISTRYINDEX); /* pop the previously pushed value. */ + + return 0; +} + +__LJMP static int hlua_applet_http_get_priv(lua_State *L) +{ + struct hlua_appctx *appctx = MAY_LJMP(hlua_checkapplet_http(L, 1)); + struct stream *s = appctx->htxn.s; + struct hlua *hlua = &s->hlua; + + /* Push configuration index in the stack. */ + lua_rawgeti(L, LUA_REGISTRYINDEX, hlua->Mref); + + return 1; +} + +>>>>>>> c43224e... BUG/MEDIUM: lua: memory leak /* If expected data not yet available, it returns a yield. This function * consumes the data in the buffer. It returns a string containing the * data. This string can be empty. @@ -4646,7 +4680,7 @@ __LJMP static int hlua_set_priv(lua_State *L) /* Remove previous value. */ if (hlua->Mref != -1) - luaL_unref(L, hlua->Mref, LUA_REGISTRYINDEX); + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); /* Get and store new value. */ lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ -- 1.7.10.4
>From 355d9493bcc738c7ae5fb0639511a47612416505 Mon Sep 17 00:00:00 2001 From: Thierry FOURNIER <[email protected]> Date: Wed, 26 Apr 2017 14:25:58 +0200 Subject: [PATCH 1/2] BUG/MEDIUM: lua: memory leak The priv context is not cleaned when we set a new priv context. This is caused by a stupid swap between two parameter of the luaL_unref() function. workaround: use set_priv only once when we process a stream. This patch should be backported in version 1.7 and 1.6 --- src/hlua.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hlua.c b/src/hlua.c index 77e5a9d..6ce18e1 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -3310,7 +3310,7 @@ __LJMP static int hlua_applet_tcp_set_priv(lua_State *L) /* Remove previous value. */ if (hlua->Mref != -1) - luaL_unref(L, hlua->Mref, LUA_REGISTRYINDEX); + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); /* Get and store new value. */ lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ @@ -3773,7 +3773,7 @@ __LJMP static int hlua_applet_http_set_priv(lua_State *L) /* Remove previous value. */ if (hlua->Mref != -1) - luaL_unref(L, hlua->Mref, LUA_REGISTRYINDEX); + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); /* Get and store new value. */ lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ @@ -4860,7 +4860,7 @@ __LJMP static int hlua_set_priv(lua_State *L) /* Remove previous value. */ if (hlua->Mref != -1) - luaL_unref(L, hlua->Mref, LUA_REGISTRYINDEX); + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); /* Get and store new value. */ lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ -- 1.7.10.4
>From 4b9178074eef43a5d1f7ba984d34580a6ec5f3ce Mon Sep 17 00:00:00 2001 From: Thierry FOURNIER <[email protected]> Date: Wed, 26 Apr 2017 13:27:05 +0200 Subject: [PATCH 2/2] CLEANUP: lua: remove test The man of "luaL_unref" says "If ref is LUA_NOREF or LUA_REFNIL, luaL_unref does nothing.", so I remove the check. --- src/hlua.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/hlua.c b/src/hlua.c index 6ce18e1..643d3fc 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -3309,8 +3309,7 @@ __LJMP static int hlua_applet_tcp_set_priv(lua_State *L) MAY_LJMP(check_args(L, 2, "set_priv")); /* Remove previous value. */ - if (hlua->Mref != -1) - luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); /* Get and store new value. */ lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ @@ -3772,8 +3771,7 @@ __LJMP static int hlua_applet_http_set_priv(lua_State *L) MAY_LJMP(check_args(L, 2, "set_priv")); /* Remove previous value. */ - if (hlua->Mref != -1) - luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); /* Get and store new value. */ lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ @@ -4859,8 +4857,7 @@ __LJMP static int hlua_set_priv(lua_State *L) hlua = hlua_gethlua(L); /* Remove previous value. */ - if (hlua->Mref != -1) - luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); + luaL_unref(L, LUA_REGISTRYINDEX, hlua->Mref); /* Get and store new value. */ lua_pushvalue(L, 2); /* Copy the element 2 at the top of the stack. */ -- 1.7.10.4
