From 655ceafd7c5c4c69d1e0668e7927882168db1d90 Mon Sep 17 00:00:00 2001 From: Peter Chou Date: Thu, 17 Mar 2022 13:25:10 -0700 Subject: [PATCH] Fixes Issue #8728 - Lua plugin memory leak on remap configuration reloads This fix adds reference counting for the Lua plugin remap instance handles. The reference counting allows us to eliminate an existing memory leak of the instance handles. In addition, this means that the old Lua memory allocated by LuaJIT may also be freed via LuaJIT garbage collection. This fix also adds the '--ljgc' remap instance plugin parameter to the Lua plugin. This paramter enables on-demand LuaJIT garbage collection while the remap instances are created and deleted. This is useful when operating close to the LuaJIT memory limit, which is currently 2GB on Linux using LuaJIT v2.1.0-beta3 from 2017. --- doc/admin-guide/plugins/lua.en.rst | 24 +++++++++++++++++++++- plugins/lua/ts_lua.c | 23 ++++++++++++++++++--- plugins/lua/ts_lua_common.h | 2 ++ plugins/lua/ts_lua_util.c | 33 ++++++++++++++++++++++++++++-- 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/doc/admin-guide/plugins/lua.en.rst b/doc/admin-guide/plugins/lua.en.rst index 02b6421c25f..8ba07fbad81 100644 --- a/doc/admin-guide/plugins/lua.en.rst +++ b/doc/admin-guide/plugins/lua.en.rst @@ -155,7 +155,29 @@ adding a configuration option to records.config. CONFIG proxy.config.plugin.lua.max_states INT 64 -Any per plugin --states value overrides this default value but must be less than or equal to this value. This setting is not reloadable since it must be applied when all the lua states are first initialized. +Any per plugin --states value overrides this default value but must be less than or equal to this value. This setting is not +reloadable since it must be applied when all the lua states are first initialized. + +For remap instances, the LuaJIT garbage collector can be set to be called automatically whenever a remap instance is created +or deleted. This happens when the remap.config file has been modified, and the configuration has been reloaded. This does +not apply to global plugin instances since these exist for the life-time of the ATS process, i.e., they are not reloadable or +reconfigurable by modifying plugin.config while ATS is running. + +By default, the LuaJIT garbage collector will run on its own according to its own internal criteria. However, in some cases, +the garbage collector should be run in a guaranteed fashion. + +For example, in Linux, total Lua memory may be limited to 2GB depending on the LuaJIT version. It may be required to release +memory on demand in order to prevent out of memory errors when running close to the memory limit. Note that the memory usage +is doubled during configuration reloads since the ATS must hold both the current and new configurations during the +transition. If garbage collection occurs does not occur immediately, memory usage may exceed this double usage. + +On demand garbage collection can be enabled by adding the following to each remap line. A value of '1' means +enabled. The default value of '0' means disabled. + +:: + + map http://a.tbcdn.cn/ http://inner.tbcdn.cn/ @plugin=/XXX/tslua.so @pparam=--ljgc=1 + Configuration for JIT mode ========================== diff --git a/plugins/lua/ts_lua.c b/plugins/lua/ts_lua.c index ea9f8aed9ee..430e73505ee 100644 --- a/plugins/lua/ts_lua.c +++ b/plugins/lua/ts_lua.c @@ -343,9 +343,11 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s char *inline_script = ""; int fn = 0; int states = ts_lua_max_state_count; + int ljgc = 0; static const struct option longopt[] = { {"states", required_argument, 0, 's'}, {"inline", required_argument, 0, 'i'}, + {"ljgc", required_argument, 0, 'g'}, {0, 0, 0, 0}, }; @@ -364,6 +366,10 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s break; case 'i': inline_script = optarg; + break; + case 'g': + ljgc = atoi(optarg); + break; } if (opt == -1) { @@ -424,6 +430,10 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s conf->states = states; conf->remap = 1; conf->init_func = 0; + conf->ref_count = 1; + conf->ljgc = ljgc; + + TSDebug(TS_LUA_DEBUG_TAG, "Reference Count = %d , creating new instance...", conf->ref_count); if (fn) { snprintf(conf->script, TS_LUA_MAX_SCRIPT_FNAME_LENGTH, "%s", script); @@ -446,6 +456,9 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s ts_lua_script_register(ts_lua_main_ctx_array[0].lua, conf->script, conf); TSMutexUnlock(ts_lua_main_ctx_array[0].mutexp); } + } else { + conf->ref_count++; + TSDebug(TS_LUA_DEBUG_TAG, "Reference Count = %d , reference existing instance...", conf->ref_count); } *ih = conf; @@ -459,9 +472,13 @@ TSRemapDeleteInstance(void *ih) int states = ((ts_lua_instance_conf *)ih)->states; ts_lua_del_module((ts_lua_instance_conf *)ih, ts_lua_main_ctx_array, states); ts_lua_del_instance(ih); - // because we now reuse ts_lua_instance_conf / ih for remap rules sharing the same lua script - // we cannot safely free it in this function during the configuration reloads - // we therefore are leaking memory on configuration reloads + ((ts_lua_instance_conf *)ih)->ref_count--; + if (((ts_lua_instance_conf *)ih)->ref_count == 0) { + TSDebug(TS_LUA_DEBUG_TAG, "Reference Count = %d , freeing...", ((ts_lua_instance_conf *)ih)->ref_count); + TSfree(ih); + } else { + TSDebug(TS_LUA_DEBUG_TAG, "Reference Count = %d , not freeing...", ((ts_lua_instance_conf *)ih)->ref_count); + } return; } diff --git a/plugins/lua/ts_lua_common.h b/plugins/lua/ts_lua_common.h index d076ae13db5..c39d03990a2 100644 --- a/plugins/lua/ts_lua_common.h +++ b/plugins/lua/ts_lua_common.h @@ -96,6 +96,8 @@ typedef struct { int remap; int states; + int ljgc; + int ref_count; int init_func; } ts_lua_instance_conf; diff --git a/plugins/lua/ts_lua_util.c b/plugins/lua/ts_lua_util.c index 3d03aeeac4e..e2cb68b89f8 100644 --- a/plugins/lua/ts_lua_util.c +++ b/plugins/lua/ts_lua_util.c @@ -367,6 +367,13 @@ ts_lua_add_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n, int a lua_newtable(L); lua_replace(L, LUA_GLOBALSINDEX); /* L[GLOBAL] = EMPTY */ + if (conf->ljgc > 0) { + TSDebug(TS_LUA_DEBUG_TAG, "ljgc = %d, running LuaJIT Garbage Collector...", conf->ljgc); + lua_gc(L, LUA_GCCOLLECT, 0); + } else { + TSDebug(TS_LUA_DEBUG_TAG, "ljgc = %d, NOT running LuaJIT Garbage Collector...", conf->ljgc); + } + TSMutexUnlock(arr[i].mutexp); } @@ -401,12 +408,27 @@ ts_lua_del_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n) } lua_pushlightuserdata(L, conf); - lua_pushvalue(L, LUA_GLOBALSINDEX); - lua_rawset(L, LUA_REGISTRYINDEX); /* L[REG][conf] = L[GLOBAL] */ + + if (conf->ref_count > 1) { + TSDebug(TS_LUA_DEBUG_TAG, "Reference Count = %d , NOT clearing registry...", conf->ref_count); + lua_pushvalue(L, LUA_GLOBALSINDEX); + lua_rawset(L, LUA_REGISTRYINDEX); /* L[REG][conf] = L[GLOBAL] */ + } else { + TSDebug(TS_LUA_DEBUG_TAG, "Reference Count = %d , clearing registry...", conf->ref_count); + lua_pushnil(L); + lua_rawset(L, LUA_REGISTRYINDEX); /* L[REG][conf] = nil */ + } lua_newtable(L); lua_replace(L, LUA_GLOBALSINDEX); /* L[GLOBAL] = EMPTY */ + if (conf->ljgc > 0) { + TSDebug(TS_LUA_DEBUG_TAG, "ljgc = %d, running LuaJIT Garbage Collector...", conf->ljgc); + lua_gc(L, LUA_GCCOLLECT, 0); + } else { + TSDebug(TS_LUA_DEBUG_TAG, "ljgc = %d, NOT running LuaJIT Garbage Collector...", conf->ljgc); + } + TSMutexUnlock(arr[i].mutexp); } @@ -468,6 +490,13 @@ ts_lua_reload_module(ts_lua_instance_conf *conf, ts_lua_main_ctx *arr, int n) lua_newtable(L); lua_replace(L, LUA_GLOBALSINDEX); /* L[GLOBAL] = EMPTY */ + if (conf->ljgc > 0) { + TSDebug(TS_LUA_DEBUG_TAG, "ljgc = %d, running LuaJIT Garbage Collector...", conf->ljgc); + lua_gc(L, LUA_GCCOLLECT, 0); + } else { + TSDebug(TS_LUA_DEBUG_TAG, "ljgc = %d, NOT running LuaJIT Garbage Collector...", conf->ljgc); + } + TSMutexUnlock(arr[i].mutexp); }