Skip to content

Commit

Permalink
Lua plugin memory leak on remap configuration reloads (#8764)
Browse files Browse the repository at this point in the history
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.

Fixes #8728
  • Loading branch information
pbchou authored Mar 30, 2022
1 parent 4fa1c0c commit b6f83f1
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 6 deletions.
24 changes: 23 additions & 1 deletion doc/admin-guide/plugins/lua.en.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
==========================
Expand Down
23 changes: 20 additions & 3 deletions plugins/lua/ts_lua.c
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};

Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
2 changes: 2 additions & 0 deletions plugins/lua/ts_lua_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ typedef struct {

int remap;
int states;
int ljgc;
int ref_count;

int init_func;
} ts_lua_instance_conf;
Expand Down
33 changes: 31 additions & 2 deletions plugins/lua/ts_lua_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit b6f83f1

Please sign in to comment.