From 26fe874ce87ad6046239750654ce111cb37a53a7 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Sun, 22 Mar 2026 16:15:37 +0200 Subject: [PATCH 01/18] Add Static Module Support Add a build option to compile the Lua scripting engine as a static module and wire the server to load it directly at startup when enabled. The module load path now resolves on-load and on-unload entry points from the main binary, and the module lifecycle keeps those callbacks so unload works without a shared library handle. The Lua module build was updated to support both static and shared variants, with the static path exporting visible wrapper symbols and linking the server with the module archive. While touching the Lua code, a few internal symbols were renamed for consistency and the monotonic time helper was clarified. * CMake * Makefile * Lua scripting module * Core module loading **Generated by CodeLite** Signed-off-by: Eran Ifrah --- CMakeLists.txt | 2 + src/CMakeLists.txt | 37 ++++--- src/Makefile | 17 ++- src/config.c | 1 + src/module.c | 182 +++++++++++++++++++++++++++++++-- src/module.h | 5 + src/modules/lua/CMakeLists.txt | 28 +++-- src/modules/lua/Makefile | 24 ++++- src/modules/lua/engine_lua.c | 25 ++++- src/modules/lua/function_lua.c | 4 +- src/modules/lua/script_lua.c | 8 +- src/server.c | 8 ++ src/server.h | 2 +- 13 files changed, 293 insertions(+), 50 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4aa26cfff8e..71d01b4ee9a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,6 +14,7 @@ endif () # Options option(BUILD_LUA "Build Valkey Lua scripting engine" ON) +option(BUILD_STATIC_LUA "Build Valkey Lua scripting engine as a static module" OFF) option(BUILD_UNIT_GTESTS "Build valkey-unit-gtests" OFF) option(BUILD_TEST_MODULES "Build all test modules" OFF) option(BUILD_EXAMPLE_MODULES "Build example modules" OFF) @@ -35,6 +36,7 @@ include(Packaging) # Clear cached variables from the cache unset(BUILD_LUA CACHE) +unset(BUILD_STATIC_LUA CACHE) unset(BUILD_TESTS CACHE) unset(CLANGPP CACHE) unset(CLANG CACHE) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 98c963153a0..1939fdbb4df 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -19,22 +19,32 @@ add_dependencies(valkey-server release_header) if (BUILD_LUA) message(STATUS "Build Lua scripting engine module") + if (BUILD_STATIC_LUA) + add_compile_definitions(STATIC_LUA=1) + message(STATUS "Building LUA as a STATIC module") + else () + add_compile_definitions(STATIC_LUA=0) + message(STATUS "Building LUA as a DYNAMIC module") + endif () add_subdirectory(modules/lua) - add_dependencies(valkey-server valkeylua) + if (BUILD_STATIC_LUA) + target_link_libraries(valkey-server $) + else () + add_dependencies(valkey-server valkeylua) + endif () target_compile_definitions(valkey-server PRIVATE LUA_ENABLED) if (UNIX AND NOT APPLE) target_compile_definitions(valkey-server PRIVATE LUA_LIB=libvalkeylua.so) - target_link_options(valkey-server PRIVATE -Wl,--disable-new-dtags) else () target_compile_definitions(valkey-server PRIVATE LUA_LIB=libvalkeylua.dylib) endif () set(VALKEY_INSTALL_RPATH "") - set_target_properties(valkey-server PROPERTIES - INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR};${CMAKE_LIBRARY_OUTPUT_DIRECTORY}" - INSTALL_RPATH_USE_LINK_PATH TRUE - BUILD_WITH_INSTALL_RPATH TRUE - ) -endif() + set_target_properties( + valkey-server + PROPERTIES INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR};${CMAKE_LIBRARY_OUTPUT_DIRECTORY}" + INSTALL_RPATH_USE_LINK_PATH TRUE + BUILD_WITH_INSTALL_RPATH TRUE) +endif () unset(BUILD_LUA CACHE) if (VALKEY_RELEASE_BUILD) @@ -49,9 +59,8 @@ if (DEBUG_FORCE_DEFRAG) endif () if (BUILD_SANITIZER) - # 'BUILD_SANITIZER' is defined in ValkeySetup module (based on user input) - # If defined, the variables 'VALKEY_SANITAIZER_CFLAGS' and 'VALKEY_SANITAIZER_LDFLAGS' - # are set with the link & compile flags required + # 'BUILD_SANITIZER' is defined in ValkeySetup module (based on user input) If defined, the variables + # 'VALKEY_SANITAIZER_CFLAGS' and 'VALKEY_SANITAIZER_LDFLAGS' are set with the link & compile flags required message(STATUS "Adding sanitizer flags for target valkey-server") target_compile_options(valkey-server PRIVATE ${VALKEY_SANITAIZER_CFLAGS}) target_link_options(valkey-server PRIVATE ${VALKEY_SANITAIZER_LDFLAGS}) @@ -118,10 +127,10 @@ endif () # Friendly hint like the Makefile one file(RELATIVE_PATH _CMAKE_DIR_RELATIVE_PATH "${CMAKE_SOURCE_DIR}" "${CMAKE_BINARY_DIR}") -add_custom_target(hint ALL +add_custom_target( + hint ALL DEPENDS valkey-server valkey-cli valkey-benchmark COMMAND ${CMAKE_COMMAND} -E echo "" COMMAND ${CMAKE_COMMAND} -E echo "Hint: It is a good idea to run tests with your CMake-built binaries \\;\\)" COMMAND ${CMAKE_COMMAND} -E echo " ./${_CMAKE_DIR_RELATIVE_PATH}/runtest" - COMMAND ${CMAKE_COMMAND} -E echo "" -) + COMMAND ${CMAKE_COMMAND} -E echo "") diff --git a/src/Makefile b/src/Makefile index 73815ac2bcf..a8b1f1a0f19 100644 --- a/src/Makefile +++ b/src/Makefile @@ -272,6 +272,19 @@ else endif endif +# Build Lua module as static library instead of shared +ifeq ($(STATIC_LUA),yes) + FINAL_CFLAGS+=-DSTATIC_LUA=1 + ifeq ($(uname_S),Darwin) + LUA_LDFLAGS=-Wl,-export_dynamic -Wl,-force_load,$(current_dir)/modules/lua/libvalkeylua.a ../deps/lua/src/liblua.a + else + LUA_LDFLAGS=-Wl,--whole-archive $(current_dir)/modules/lua/libvalkeylua.a -Wl,--no-whole-archive ../deps/lua/src/liblua.a + endif +else + FINAL_CFLAGS+=-DSTATIC_LUA=0 + LUA_LDFLAGS= +endif + # Determine systemd support and/or build preference (defaulting to auto-detection) BUILD_WITH_SYSTEMD=no LIBSYSTEMD_LIBS=-lsystemd @@ -688,7 +701,7 @@ endif # valkey-server $(SERVER_NAME): $(ENGINE_SERVER_OBJ) $(LUA_MODULE) - $(SERVER_LD) -o $@ $(ENGINE_SERVER_OBJ) ../deps/libvalkey/lib/libvalkey.a ../deps/hdr_histogram/libhdrhistogram.a ../deps/fpconv/libfpconv.a $(FINAL_LIBS) + $(SERVER_LD) -o $@ $(ENGINE_SERVER_OBJ) ../deps/libvalkey/lib/libvalkey.a ../deps/hdr_histogram/libhdrhistogram.a ../deps/fpconv/libfpconv.a $(FINAL_LIBS) $(LUA_LDFLAGS) # Valkey static library, used to compile against for unit testing $(ENGINE_LIB_NAME): $(ENGINE_SERVER_OBJ) @@ -716,7 +729,7 @@ $(RDMA_MODULE_NAME): $(SERVER_NAME) # engine_lua.so $(LUA_MODULE_NAME): .make-prerequisites - cd modules/lua && $(MAKE) OPTIMIZATION="$(OPTIMIZATION)" + $(MAKE) -C modules/lua OPTIMIZATION="$(OPTIMIZATION)" STATIC_LUA="$(STATIC_LUA)" # valkey-cli $(ENGINE_CLI_NAME): $(ENGINE_CLI_OBJ) diff --git a/src/config.c b/src/config.c index 93ef289e328..8bd94ed0c0d 100644 --- a/src/config.c +++ b/src/config.c @@ -1617,6 +1617,7 @@ void rewriteConfigLoadmoduleOption(struct rewriteConfigState *state) { dictEntry *de; while ((de = dictNext(di)) != NULL) { struct ValkeyModule *module = dictGetVal(de); + if (module->is_static_module) continue; line = moduleLoadQueueEntryToLoadmoduleOptionStr(module, "loadmodule"); rewriteConfigRewriteLine(state, "loadmodule", line, 1); } diff --git a/src/module.c b/src/module.c index de5a5510e40..aae2db72c1c 100644 --- a/src/module.c +++ b/src/module.c @@ -55,18 +55,17 @@ * replacements are done, such as the replacement of RM with ValkeyModule in * function names. For details, see the script src/modules/gendoc.rb. * -------------------------------------------------------------------------- */ - #include "server.h" #include "cluster.h" #include "commandlog.h" #include "rdb.h" #include "monotonic.h" #include "script.h" -#include "call_reply.h" #include "hdr_histogram.h" #include "crc16_slottable.h" #include "valkeymodule.h" #include "module.h" +#include "call_reply.h" #include "io_threads.h" #include "scripting_engine.h" #include "cluster_migrateslots.h" @@ -12889,6 +12888,7 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa dictAdd(modules, ctx.module->name, ctx.module); ctx.module->blocked_clients = 0; ctx.module->handle = handle; + ctx.module->is_static_module = 0; ctx.module->loadmod = zmalloc(sizeof(struct moduleLoadQueueEntry)); ctx.module->loadmod->path = sdsnew(path); ctx.module->loadmod->argv = module_argc ? zmalloc(sizeof(robj *) * module_argc) : NULL; @@ -12932,6 +12932,162 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa return C_OK; } +/** + * Load a static symbol from the current process by constructing a module-specific symbol name. + * + * This function builds the lookup name as "_", opens the current + * process image with dlopen(), and then resolves the symbol with dlsym(). On failure, it logs + * a warning and returns C_ERR; on success, it stores the resolved address and handle and returns + * C_OK. + * + * @param out + * void **. Output pointer that receives the address of the resolved symbol. + * @param handle + * void **. Output pointer that receives the dlopen() handle for the current process. + * @param symbol_name + * const char *. Base name of the symbol to resolve. + * @param module_name + * const char *. Module name appended to the symbol name to form the final lookup name. + * + * @return int + * C_OK on success, or C_ERR if the symbol name is too long, dlopen() fails, or dlsym() + * cannot resolve the symbol. + * + * @note This function is intended for use in the module-loading context and relies on the + * current process exposing statically linked module entry points under the constructed name. + */ +static int moduleLoadStaticSymbol(void **out, void **handle, const char *symbol_name, const char *module_name) { + char symbol_full_name[128]; + int n = snprintf(symbol_full_name, sizeof(symbol_full_name), "%s_%s", symbol_name, module_name); + if (n >= (int)sizeof(symbol_full_name)) { + serverLog(LL_WARNING, "Module name is too long"); + return C_ERR; + } + + // Open a handle to self + *handle = dlopen(NULL, RTLD_NOW); + if (*handle == NULL) { + char *error = dlerror(); + if (error == NULL) error = "Unknown error"; + + serverLog(LL_WARNING, "Failed to load static module: %s. %s", module_name, error); + return C_ERR; + } + + *out = dlsym(*handle, symbol_full_name); + if (*out == NULL) { + char *error = dlerror(); + if (error == NULL) error = "Unknown error"; + + serverLog(LL_WARNING, + "Failed to load static module: %s. Could not load method: %s. %s", module_name, + symbol_full_name, error); + dlclose(*handle); + return C_ERR; + } + return C_OK; +} + +/** + * @brief Loads a statically linked module by locating its on-load and on-unload entry points. + * + * This function builds the expected symbol names for the module, resolves them from the current + * process image, and invokes the module's initialization routine. On success, the module is registered + * with the server, its state is initialized, and module load events are fired. If validation or + * post-load checks fail, the module is unloaded and an error is returned. + * + * @param module_name const char * Name of the static module, used to construct the on-load and + * on-unload symbol names. + * @param module_argv void ** Array of module arguments passed through to the module's on-load + * function. + * @param module_argc int Number of entries in module_argv. + * @param is_loadex int Nonzero if the module is being loaded via the loadex path and queued + * configuration arguments should be validated. + * + * @return int C_OK on successful load; C_ERR if symbol lookup fails, module initialization fails, + * or post-load validation detects an error. + * + * @note This function operates in the server/module loading context and expects the module's + * ValkeyModule_OnLoad_ and ValkeyModule_OnUnload_ symbols to be present in the + * current process. + * + * @throws No exceptions are thrown. Errors are reported via server logging and by returning C_ERR. + */ +int moduleLoadStatic(const char *module_name, void **module_argv, int module_argc, int is_loadex) { + // Locate the load + ModuleLoadFunc onload; + void *handle = NULL; + if (moduleLoadStaticSymbol((void **)&onload, &handle, "ValkeyModule_OnLoad", module_name) != C_OK) { + return C_ERR; + } + + ValkeyModuleCtx ctx; + moduleCreateContext(&ctx, NULL, VALKEYMODULE_CTX_TEMP_CLIENT); /* We pass NULL since we don't have a module yet. */ + if (onload((void *)&ctx, module_argv, module_argc) == VALKEYMODULE_ERR) { + if (ctx.module) { + serverLog(LL_WARNING, "Static Module %s initialization failed. Module not loaded.", module_name); + moduleUnregisterCleanup(ctx.module); + moduleRemoveCateogires(ctx.module); + moduleFreeModuleStructure(ctx.module); + } else { + /* If there is no ctx.module, this means that our ValkeyModule_Init call failed, + * and currently init will only fail on busy name. */ + serverLog(LL_WARNING, "Static Module %s initialization failed. Module name is busy.", module_name); + } + moduleFreeContext(&ctx); + dlclose(handle); + return C_ERR; + } + + dlclose(handle); + + /* Module loaded! Register it. */ + dictAdd(modules, ctx.module->name, ctx.module); + ctx.module->blocked_clients = 0; + ctx.module->handle = NULL; // We will re-open when needed. + ctx.module->is_static_module = 1; + ctx.module->loadmod = zmalloc(sizeof(struct moduleLoadQueueEntry)); + ctx.module->loadmod->path = sdsnew(module_name); + ctx.module->loadmod->argv = module_argc ? zmalloc(sizeof(robj *) * module_argc) : NULL; + ctx.module->loadmod->argc = module_argc; + for (int i = 0; i < module_argc; i++) { + ctx.module->loadmod->argv[i] = module_argv[i]; + incrRefCount(ctx.module->loadmod->argv[i]); + } + + /* If module commands have ACL categories, recompute command bits + * for all existing users once the modules has been registered. */ + if (ctx.module->num_commands_with_acl_categories) { + ACLRecomputeCommandBitsFromCommandRulesAllUsers(); + } + serverLog(LL_NOTICE, "Static Module '%s' successfully loaded", ctx.module->name); + ctx.module->onload = 0; + + int post_load_err = 0; + if (listLength(ctx.module->module_configs) && !ctx.module->configs_initialized) { + serverLogRaw(LL_WARNING, + "Module Configurations were not set, likely a missing LoadConfigs call. Unloading the module."); + post_load_err = 1; + } + + if (is_loadex && dictSize(server.module_configs_queue)) { + serverLogRaw(LL_WARNING, + "Loadex configurations were not applied, likely due to invalid arguments. Unloading the module."); + post_load_err = 1; + } + + if (post_load_err) { + moduleUnload(ctx.module->name, NULL); + moduleFreeContext(&ctx); + return C_ERR; + } + + /* Fire the loaded modules event. */ + moduleFireServerEvent(VALKEYMODULE_EVENT_MODULE_CHANGE, VALKEYMODULE_SUBEVENT_MODULE_LOADED, ctx.module); + moduleFreeContext(&ctx); + return C_OK; +} + static int moduleUnloadInternal(struct ValkeyModule *module, const char **errmsg) { if (listLength(module->types)) { *errmsg = "the module exports one or more module-side data " @@ -12964,15 +13120,21 @@ static int moduleUnloadInternal(struct ValkeyModule *module, const char **errmsg } /* Give module a chance to clean up. */ - const char *onUnloadNames[] = {"ValkeyModule_OnUnload", "RedisModule_OnUnload"}; - int (*onunload)(void *) = NULL; - for (size_t i = 0; i < sizeof(onUnloadNames) / sizeof(onUnloadNames[0]); i++) { - onunload = (int (*)(void *))(unsigned long)dlsym(module->handle, onUnloadNames[i]); - if (onunload) { - if (i != 0) { - serverLog(LL_NOTICE, "Legacy Redis Module %s found", module->name); + ModuleUnLoadFunc onunload = NULL; + if (module->is_static_module == 1) { + if (moduleLoadStaticSymbol((void **)&onunload, &module->handle, "ValkeyModule_OnUnload", module->name) != C_OK) { + return C_ERR; + } + } else { + const char *onUnloadNames[] = {"ValkeyModule_OnUnload", "RedisModule_OnUnload"}; + for (size_t i = 0; i < sizeof(onUnloadNames) / sizeof(onUnloadNames[0]); i++) { + onunload = (int (*)(void *))(unsigned long)dlsym(module->handle, onUnloadNames[i]); + if (onunload) { + if (i != 0) { + serverLog(LL_NOTICE, "Legacy Redis Module %s found", module->name); + } + break; } - break; } } diff --git a/src/module.h b/src/module.h index c7ad384c6a1..91daac94c60 100644 --- a/src/module.h +++ b/src/module.h @@ -95,6 +95,9 @@ typedef struct moduleValue { void *value; } moduleValue; +typedef int (*ModuleLoadFunc)(void *, void **, int); +typedef int (*ModuleUnLoadFunc)(void *); + /* This structure represents a module inside the system. */ typedef struct ValkeyModule { void *handle; /* Module dlopen() handle. */ @@ -117,6 +120,7 @@ typedef struct ValkeyModule { int num_commands_with_acl_categories; /* Number of commands in this module included in acl categories */ int onload; /* Flag to identify if the call is being made from Onload (0 or 1) */ size_t num_acl_categories_added; /* Number of acl categories added by this module. */ + int is_static_module; /* 1 if this is a static module, 0 otherwise */ } ValkeyModule; /* This is a wrapper for the 'rio' streams used inside rdb.c in the server, so that @@ -182,6 +186,7 @@ void moduleInitModulesSystem(void); void moduleInitModulesSystemLast(void); void modulesCron(void); int moduleLoad(const char *path, void **argv, int argc, int is_loadex); +int moduleLoadStatic(const char *path, void **argv, int argc, int is_loadex); int moduleUnload(sds name, const char **errmsg); void moduleUnloadAllModules(void); void moduleLoadFromQueue(void); diff --git a/src/modules/lua/CMakeLists.txt b/src/modules/lua/CMakeLists.txt index 00498d1a0b8..1057abf6dbd 100644 --- a/src/modules/lua/CMakeLists.txt +++ b/src/modules/lua/CMakeLists.txt @@ -4,22 +4,30 @@ if (VALKEY_DEBUG_BUILD) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -W -Wall -fno-common -g -ggdb -std=c99 -O2 -D_GNU_SOURCE") else () set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -W -Wall -fno-common -O3 -std=c99 -D_GNU_SOURCE") -endif() +endif () set(LUA_ENGINE_SRCS engine_lua.c script_lua.c function_lua.c debug_lua.c - list.c - ../../sha1.c - ../../rand.c) + list.c) -add_library(valkeylua SHARED "${LUA_ENGINE_SRCS}") +if (NOT BUILD_STATIC_LUA) + list(APPEND LUA_ENGINE_SRCS ../../sha1.c) + list(APPEND LUA_ENGINE_SRCS ../../rand.c) +endif () -target_link_libraries(valkeylua PRIVATE lualib fpconv) -target_include_directories(valkeylua PRIVATE ../../../deps/lua/src) +if (BUILD_STATIC_LUA) + message(STATUS "Building STATIC LUA module") + add_library(valkeylua STATIC "${LUA_ENGINE_SRCS}") + target_link_libraries(valkeylua PUBLIC lualib fpconv) + target_include_directories(valkeylua PUBLIC ../../../deps/lua/src) +else () + message(STATUS "Building DYNAMIC LUA module") + add_library(valkeylua SHARED "${LUA_ENGINE_SRCS}") + target_link_libraries(valkeylua PRIVATE lualib fpconv) + target_include_directories(valkeylua PRIVATE ../../../deps/lua/src) +endif () -install(TARGETS valkeylua - LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} -) +install(TARGETS valkeylua LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/src/modules/lua/Makefile b/src/modules/lua/Makefile index 830461ddb4f..b64d9af8086 100644 --- a/src/modules/lua/Makefile +++ b/src/modules/lua/Makefile @@ -3,11 +3,17 @@ CLANG := $(findstring clang,$(shell sh -c '$(CC) --version | head -1')) DEPS_DIR=../../../deps +ifeq ($(STATIC_LUA),yes) + STATIC_LUA_FLAG=-DSTATIC_LUA=1 +else + STATIC_LUA_FLAG=-DSTATIC_LUA=0 +endif + ifeq ($(uname_S),Darwin) - SHOBJ_CFLAGS= -I. -I$(DEPS_DIR)/lua/src -I$(DEPS_DIR)/fpconv -fPIC -W -Wall -dynamic -fno-common $(OPTIMIZATION) -std=gnu11 -D_GNU_SOURCE $(CFLAGS) + SHOBJ_CFLAGS= -I. -I$(DEPS_DIR)/lua/src -I$(DEPS_DIR)/fpconv -fPIC -W -Wall -dynamic -fno-common $(OPTIMIZATION) -std=gnu11 -D_GNU_SOURCE $(STATIC_LUA_FLAG) $(CFLAGS) SHOBJ_LDFLAGS= -bundle -undefined dynamic_lookup $(LDFLAGS) else - SHOBJ_CFLAGS= -I. -I$(DEPS_DIR)/lua/src -I$(DEPS_DIR)/fpconv -fPIC -W -Wall -fno-common $(OPTIMIZATION) -std=gnu11 -D_GNU_SOURCE $(CFLAGS) + SHOBJ_CFLAGS= -I. -I$(DEPS_DIR)/lua/src -I$(DEPS_DIR)/fpconv -fPIC -W -Wall -fno-common $(OPTIMIZATION) -std=gnu11 -D_GNU_SOURCE $(STATIC_LUA_FLAG) $(CFLAGS) SHOBJ_LDFLAGS= -shared $(LDFLAGS) # Pretty-printing setup for module build @@ -43,10 +49,15 @@ LIBS = \ $(DEPS_DIR)/lua/src/liblua.a \ $(DEPS_DIR)/fpconv/libfpconv.a SRCS= $(wildcard *.c) + +ifeq ($(STATIC_LUA),yes) +OBJS = $(SRCS:.c=.o) +else OBJS = \ $(SRCS:.c=.o) \ sha1.o \ rand.o +endif # OS X 11.x doesn't have /usr/lib/libSystem.dylib and needs an explicit setting. ifeq ($(uname_S),Darwin) @@ -55,10 +66,17 @@ ifeq ("$(wildcard /usr/lib/libSystem.dylib)","") endif endif +ifeq ($(STATIC_LUA),yes) +all: libvalkeylua.a + +libvalkeylua.a: $(OBJS) $(LIBS) + $(QUIET_LINK_MOD)$(AR) rcs $@ $(OBJS) +else all: libvalkeylua.so libvalkeylua.so: $(OBJS) $(LIBS) $(QUIET_LINK_MOD)$(CC) -o $@ $(SHOBJ_LDFLAGS) $^ +endif sha1.o: ../../sha1.c $(QUIET_CC_MOD)$(CC) $(SHOBJ_CFLAGS) -c $< -o $@ @@ -76,4 +94,4 @@ $(DEPS_DIR)/fpconv/libfpconv.a: cd $(DEPS_DIR) && $(MAKE) fpconv clean: - rm -f *.so $(OBJS) + rm -f *.so *.a $(OBJS) diff --git a/src/modules/lua/engine_lua.c b/src/modules/lua/engine_lua.c index afdc4e20f32..5141ef9561e 100644 --- a/src/modules/lua/engine_lua.c +++ b/src/modules/lua/engine_lua.c @@ -480,9 +480,15 @@ static void luaEngineDebuggerEnd(ValkeyModuleCtx *module_ctx, static struct luaEngineCtx *engine_ctx = NULL; -int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, - ValkeyModuleString **argv, - int argc) { +#if STATIC_LUA +#define LUA_MODULE_VISIBILITY static +#else +#define LUA_MODULE_VISIBILITY +#endif + +LUA_MODULE_VISIBILITY int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, + ValkeyModuleString **argv, + int argc) { VALKEYMODULE_NOT_USED(argv); VALKEYMODULE_NOT_USED(argc); @@ -533,7 +539,8 @@ int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, return VALKEYMODULE_OK; } -int ValkeyModule_OnUnload(ValkeyModuleCtx *ctx) { + +LUA_MODULE_VISIBILITY int ValkeyModule_OnUnload(ValkeyModuleCtx *ctx) { if (ValkeyModule_UnregisterScriptingEngine(ctx, LUA_ENGINE_NAME) != VALKEYMODULE_OK) { ValkeyModule_Log(ctx, "error", "Failed to unregister engine"); return VALKEYMODULE_ERR; @@ -544,3 +551,13 @@ int ValkeyModule_OnUnload(ValkeyModuleCtx *ctx) { return VALKEYMODULE_OK; } + +__attribute__((visibility("default"))) int ValkeyModule_OnLoad_lua(ValkeyModuleCtx *ctx, + ValkeyModuleString **argv, + int argc) { + return ValkeyModule_OnLoad(ctx, argv, argc); +} + +__attribute__((visibility("default"))) int ValkeyModule_OnUnload_lua(ValkeyModuleCtx *ctx) { + return ValkeyModule_OnUnload(ctx); +} diff --git a/src/modules/lua/function_lua.c b/src/modules/lua/function_lua.c index 3dd9b169d30..dbabd9a37d9 100644 --- a/src/modules/lua/function_lua.c +++ b/src/modules/lua/function_lua.c @@ -213,7 +213,7 @@ typedef struct flagStr { const char *str; } flagStr; -flagStr scripts_flags_def[] = { +flagStr lua_scripts_flags_def[] = { {.flag = VMSE_SCRIPT_FLAG_NO_WRITES, .str = "no-writes"}, {.flag = VMSE_SCRIPT_FLAG_ALLOW_OOM, .str = "allow-oom"}, {.flag = VMSE_SCRIPT_FLAG_ALLOW_STALE, .str = "allow-stale"}, @@ -244,7 +244,7 @@ static int luaRegisterFunctionReadFlags(lua_State *lua, uint64_t *flags) { const char *flag_str = lua_tostring(lua, -1); int found = 0; - for (flagStr *flag = scripts_flags_def; flag->str; ++flag) { + for (flagStr *flag = lua_scripts_flags_def; flag->str; ++flag) { if (!strcasecmp(flag->str, flag_str)) { f_flags |= flag->flag; found = 1; diff --git a/src/modules/lua/script_lua.c b/src/modules/lua/script_lua.c index f8d7faac900..d92f7a2be4c 100644 --- a/src/modules/lua/script_lua.c +++ b/src/modules/lua/script_lua.c @@ -172,7 +172,7 @@ static void _serverPanic(const char *file, int line, const char *msg, ...) { typedef uint64_t monotime; -monotime getMonotonicUs(void) { +monotime getMonotonicMicroseconds(void) { /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems * did not support this until much later. CLOCK_MONOTONIC is technically * optional and may not be supported - but it appears to be universal. @@ -183,7 +183,7 @@ monotime getMonotonicUs(void) { } inline uint64_t elapsedUs(monotime start_time) { - return getMonotonicUs() - start_time; + return getMonotonicMicroseconds() - start_time; } inline uint64_t elapsedMs(monotime start_time) { @@ -1162,7 +1162,7 @@ static int luaRedisPCallCommand(lua_State *lua) { * * 'digest' should point to a 41 bytes buffer: 40 for SHA1 converted into an * hexadecimal number, plus 1 byte for null term. */ -void sha1hex(char *digest, char *script, size_t len) { +void sha1Hex(char *digest, char *script, size_t len) { SHA1_CTX ctx; unsigned char hash[20]; char *cset = "0123456789abcdef"; @@ -1193,7 +1193,7 @@ static int luaRedisSha1hexCommand(lua_State *lua) { } s = (char *)lua_tolstring(lua, 1, &len); - sha1hex(digest, s, len); + sha1Hex(digest, s, len); lua_pushstring(lua, digest); return 1; } diff --git a/src/server.c b/src/server.c index 69d0cd27ab0..30eb98e6795 100644 --- a/src/server.c +++ b/src/server.c @@ -7627,6 +7627,13 @@ __attribute__((weak)) int main(int argc, char **argv) { /* Initialize the LUA scripting engine. */ #ifdef LUA_ENABLED +#if STATIC_LUA + if (scriptingEngineManagerFind("lua") == NULL) { + if (moduleLoadStatic("lua", NULL, 0, 0) != C_OK) { + serverPanic("Lua engine initialization failed, check the server logs."); + } + } +#else #define LUA_LIB_STR STRINGIFY(LUA_LIB) if (scriptingEngineManagerFind("lua") == NULL) { if (moduleLoad(LUA_LIB_STR, NULL, 0, 0) != C_OK) { @@ -7634,6 +7641,7 @@ __attribute__((weak)) int main(int argc, char **argv) { } } #endif +#endif InitServerLast(); diff --git a/src/server.h b/src/server.h index b306db549e9..5897489b1c2 100644 --- a/src/server.h +++ b/src/server.h @@ -1445,7 +1445,7 @@ bool isImportSlotMigrationJob(slotMigrationJob *job); * CLIENT_TYPE_PUBSUB -> Client subscribed to Pub/Sub channels * CLIENT_TYPE_PRIMARY -> The client representing our replication primary. */ -static inline int getClientType(client *c) { +static inline __attribute__((always_inline)) int getClientType(client *c) { if (unlikely(c->flag.primary)) return CLIENT_TYPE_PRIMARY; /* Even though MONITOR clients are marked as replicas, we * want the expose them as normal clients. */ From df6c34ffc348c4d7664f8ccc1081145f21f8286a Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Wed, 25 Mar 2026 17:49:56 +0200 Subject: [PATCH 02/18] Fix Remove Cached Eval Scripts On Engine Unregister When a scripting engine is unregistered, its cached eval scripts are now removed as well. This prevents the eval cache from holding dangling references to the engine after it has been unloaded. The eval subsystem now exposes a helper for deleting all scripts owned by a specific engine, updates the eval cache bookkeeping as entries are removed, and logs how many scripts were cleaned up during engine unregistration. * scripting_engine * eval **Generated by CodeLite** Signed-off-by: Eran Ifrah --- src/eval.c | 31 +++++++++++++++++++++++++++++++ src/eval.h | 3 +++ src/scripting_engine.c | 2 ++ 3 files changed, 36 insertions(+) diff --git a/src/eval.c b/src/eval.c index 0baba539535..1823cf35f11 100644 --- a/src/eval.c +++ b/src/eval.c @@ -156,6 +156,12 @@ void freeEvalScripts(dict *scripts, list *scripts_lru_list, list *engine_callbac } } +void freeCachedScriptsForEngine(scriptingEngine *engine) { + if (engine == NULL) { + return; + } +} + static void resetEngineEvalEnvCallback(scriptingEngine *engine, void *context) { int async = context != NULL; callableLazyEnvReset *callback = scriptingEngineCallResetEnvFunc(engine, VMSE_EVAL, async); @@ -180,6 +186,31 @@ void evalRelease(int async) { } +/* Remove all cached eval scripts associated with the given scripting engine. + * Called when a scripting engine is unregistered to avoid dangling engine + * pointers in the eval script cache. */ +void evalRemoveScriptsOfEngine(scriptingEngine *engine, const char *engine_name) { + dictIterator *iter = dictGetSafeIterator(evalCtx.scripts); + dictEntry *entry; + size_t scripts_removed = 0; + while ((entry = dictNext(iter))) { + evalScript *es = dictGetVal(entry); + if (es->engine == engine) { + sds sha = dictGetKey(entry); + evalCtx.scripts_mem -= sdsAllocSize(sha) + getStringObjectSdsUsedMemory(es->body); + if (es->node) { + listDelNode(evalCtx.scripts_lru_list, es->node); + } + dictDelete(evalCtx.scripts, sha); + ++scripts_removed; + } + } + dictReleaseIterator(iter); + serverLog(LL_NOTICE, + "Successfully removed %zu cached scripts associated with engine '%s'", + scripts_removed, engine_name); +} + void evalReset(int async) { evalRelease(async); evalInit(); diff --git a/src/eval.h b/src/eval.h index f9c217ef0da..280071ad4be 100644 --- a/src/eval.h +++ b/src/eval.h @@ -1,8 +1,11 @@ #ifndef _EVAL_H_ #define _EVAL_H_ +typedef struct scriptingEngine scriptingEngine; + void evalInit(void); void evalReset(int async); +void evalRemoveScriptsOfEngine(scriptingEngine *engine, const char *engine_name); void *evalActiveDefragScript(void *ptr); #endif /* _EVAL_H_ */ diff --git a/src/scripting_engine.c b/src/scripting_engine.c index c923e1e956f..470d9ebca57 100644 --- a/src/scripting_engine.c +++ b/src/scripting_engine.c @@ -7,6 +7,7 @@ #include "scripting_engine.h" #include "bio.h" #include "dict.h" +#include "eval.h" #include "functions.h" #include "module.h" #include "server.h" @@ -184,6 +185,7 @@ int scriptingEngineManagerUnregister(const char *engine_name) { scriptingEngine *e = dictGetVal(entry); functionsRemoveLibFromEngine(e); + evalRemoveScriptsOfEngine(e, e->name); engineMemoryInfo mem_info = scriptingEngineCallGetMemoryInfo(e, VMSE_ALL); engineMgr.total_memory_overhead -= zmalloc_size(e) + From aacd5757a7651b6ba2da04a30ca23909e0e3c178 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Wed, 25 Mar 2026 17:53:40 +0200 Subject: [PATCH 03/18] Fix Lua Monotonic Time Handling For Static Builds Update the Lua module to use the engine-provided monotonic clock helper when built with STATIC_LUA, while keeping the module's local implementation for non-static builds. This avoids duplicate timing implementations and keeps elapsed-time handling consistent across build configurations. * Lua module * Static build integration **Generated by CodeLite** Signed-off-by: Eran Ifrah --- src/modules/lua/script_lua.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/modules/lua/script_lua.c b/src/modules/lua/script_lua.c index d92f7a2be4c..f007a7d0229 100644 --- a/src/modules/lua/script_lua.c +++ b/src/modules/lua/script_lua.c @@ -172,7 +172,11 @@ static void _serverPanic(const char *file, int line, const char *msg, ...) { typedef uint64_t monotime; -monotime getMonotonicMicroseconds(void) { +#if STATIC_LUA +/* Use the engine's version */ +extern monotime getMonotonicUs(void); +#else +monotime getMonotonicUs(void) { /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems * did not support this until much later. CLOCK_MONOTONIC is technically * optional and may not be supported - but it appears to be universal. @@ -181,9 +185,10 @@ monotime getMonotonicMicroseconds(void) { clock_gettime(CLOCK_MONOTONIC, &ts); return ((uint64_t)ts.tv_sec) * 1000000 + ts.tv_nsec / 1000; } +#endif inline uint64_t elapsedUs(monotime start_time) { - return getMonotonicMicroseconds() - start_time; + return getMonotonicUs() - start_time; } inline uint64_t elapsedMs(monotime start_time) { From 114c7831d4977517973309c5adabd0e9870df788 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Wed, 25 Mar 2026 18:45:35 +0200 Subject: [PATCH 04/18] Update server startup with LUA & Refactor Lua Build Modes The server startup path now only auto-loads Lua at runtime for the static build. Consolidate Lua scripting engine build configuration around a single BUILD_LUA value with explicit static, module, and no modes. This removes the separate BUILD_STATIC_LUA toggle and updates the build system to derive compile definitions, link behavior, and module packaging from the selected mode. * Build system * Server startup * Lua module packaging **Generated by CodeLite** Signed-off-by: Eran Ifrah --- CMakeLists.txt | 4 +--- src/CMakeLists.txt | 6 +++--- src/Makefile | 28 +++++++++++++--------------- src/modules/lua/CMakeLists.txt | 4 ++-- src/modules/lua/Makefile | 30 +++++++++++++++--------------- src/server.c | 14 ++------------ 6 files changed, 36 insertions(+), 50 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 71d01b4ee9a..3c7c1b8a352 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,8 +13,7 @@ if (APPLE) endif () # Options -option(BUILD_LUA "Build Valkey Lua scripting engine" ON) -option(BUILD_STATIC_LUA "Build Valkey Lua scripting engine as a static module" OFF) +set(BUILD_LUA "static" CACHE STRING "Build Valkey Lua scripting engine: static (default), module, no") option(BUILD_UNIT_GTESTS "Build valkey-unit-gtests" OFF) option(BUILD_TEST_MODULES "Build all test modules" OFF) option(BUILD_EXAMPLE_MODULES "Build example modules" OFF) @@ -36,7 +35,6 @@ include(Packaging) # Clear cached variables from the cache unset(BUILD_LUA CACHE) -unset(BUILD_STATIC_LUA CACHE) unset(BUILD_TESTS CACHE) unset(CLANGPP CACHE) unset(CLANG CACHE) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1939fdbb4df..6ef9d816fcb 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -17,9 +17,9 @@ add_dependencies(valkey-server generate_commands_def) add_dependencies(valkey-server generate_fmtargs_h) add_dependencies(valkey-server release_header) -if (BUILD_LUA) +if (NOT BUILD_LUA STREQUAL "no") message(STATUS "Build Lua scripting engine module") - if (BUILD_STATIC_LUA) + if (BUILD_LUA STREQUAL "static") add_compile_definitions(STATIC_LUA=1) message(STATUS "Building LUA as a STATIC module") else () @@ -27,7 +27,7 @@ if (BUILD_LUA) message(STATUS "Building LUA as a DYNAMIC module") endif () add_subdirectory(modules/lua) - if (BUILD_STATIC_LUA) + if (BUILD_LUA STREQUAL "static") target_link_libraries(valkey-server $) else () add_dependencies(valkey-server valkeylua) diff --git a/src/Makefile b/src/Makefile index a8b1f1a0f19..e68a4e58851 100644 --- a/src/Makefile +++ b/src/Makefile @@ -258,31 +258,29 @@ LUA_MODULE_NAME:=modules/lua/libvalkeylua.so ifeq ($(BUILD_LUA),no) LUA_MODULE= LUA_MODULE_INSTALL= -else + FINAL_CFLAGS+=-DSTATIC_LUA=0 +else ifeq ($(BUILD_LUA),module) LUA_MODULE=$(LUA_MODULE_NAME) LUA_MODULE_INSTALL=install-lua-module current_dir = $(shell pwd) - FINAL_CFLAGS+=-DLUA_ENABLED -DLUA_LIB=libvalkeylua.so -ifeq ($(uname_S),Darwin) - FINAL_LDFLAGS+= -Wl,-rpath,$(PREFIX)/lib - FINAL_LDFLAGS+= -Wl,-rpath,$(current_dir)/modules/lua + FINAL_CFLAGS+=-DLUA_ENABLED -DLUA_LIB=libvalkeylua.so -DSTATIC_LUA=0 + ifeq ($(uname_S),Darwin) + FINAL_LDFLAGS+= -Wl,-rpath,$(PREFIX)/lib + FINAL_LDFLAGS+= -Wl,-rpath,$(current_dir)/modules/lua + else + FINAL_LDFLAGS+= -Wl,-rpath,$(PREFIX)/lib:$(current_dir)/modules/lua -Wl,--disable-new-dtags + endif else - FINAL_LDFLAGS+= -Wl,-rpath,$(PREFIX)/lib:$(current_dir)/modules/lua -Wl,--disable-new-dtags -endif -endif - -# Build Lua module as static library instead of shared -ifeq ($(STATIC_LUA),yes) - FINAL_CFLAGS+=-DSTATIC_LUA=1 + # The default: building Lua as a static module. + LUA_MODULE=$(LUA_MODULE_NAME) + current_dir = $(shell pwd) + FINAL_CFLAGS+=-DLUA_ENABLED -DSTATIC_LUA=1 ifeq ($(uname_S),Darwin) LUA_LDFLAGS=-Wl,-export_dynamic -Wl,-force_load,$(current_dir)/modules/lua/libvalkeylua.a ../deps/lua/src/liblua.a else LUA_LDFLAGS=-Wl,--whole-archive $(current_dir)/modules/lua/libvalkeylua.a -Wl,--no-whole-archive ../deps/lua/src/liblua.a endif -else - FINAL_CFLAGS+=-DSTATIC_LUA=0 - LUA_LDFLAGS= endif # Determine systemd support and/or build preference (defaulting to auto-detection) diff --git a/src/modules/lua/CMakeLists.txt b/src/modules/lua/CMakeLists.txt index 1057abf6dbd..6b8d40a3921 100644 --- a/src/modules/lua/CMakeLists.txt +++ b/src/modules/lua/CMakeLists.txt @@ -13,12 +13,12 @@ set(LUA_ENGINE_SRCS debug_lua.c list.c) -if (NOT BUILD_STATIC_LUA) +if (BUILD_LUA STREQUAL "module") list(APPEND LUA_ENGINE_SRCS ../../sha1.c) list(APPEND LUA_ENGINE_SRCS ../../rand.c) endif () -if (BUILD_STATIC_LUA) +if (BUILD_LUA STREQUAL "static") message(STATUS "Building STATIC LUA module") add_library(valkeylua STATIC "${LUA_ENGINE_SRCS}") target_link_libraries(valkeylua PUBLIC lualib fpconv) diff --git a/src/modules/lua/Makefile b/src/modules/lua/Makefile index b64d9af8086..4a486a4b3c4 100644 --- a/src/modules/lua/Makefile +++ b/src/modules/lua/Makefile @@ -3,10 +3,10 @@ CLANG := $(findstring clang,$(shell sh -c '$(CC) --version | head -1')) DEPS_DIR=../../../deps -ifeq ($(STATIC_LUA),yes) - STATIC_LUA_FLAG=-DSTATIC_LUA=1 -else +ifeq ($(BUILD_LUA),module) STATIC_LUA_FLAG=-DSTATIC_LUA=0 +else + STATIC_LUA_FLAG=-DSTATIC_LUA=1 endif ifeq ($(uname_S),Darwin) @@ -50,13 +50,13 @@ LIBS = \ $(DEPS_DIR)/fpconv/libfpconv.a SRCS= $(wildcard *.c) -ifeq ($(STATIC_LUA),yes) -OBJS = $(SRCS:.c=.o) -else +ifeq ($(BUILD_LUA),module) OBJS = \ - $(SRCS:.c=.o) \ - sha1.o \ - rand.o + $(SRCS:.c=.o) \ + sha1.o \ + rand.o +else +OBJS = $(SRCS:.c=.o) endif # OS X 11.x doesn't have /usr/lib/libSystem.dylib and needs an explicit setting. @@ -66,16 +66,16 @@ ifeq ("$(wildcard /usr/lib/libSystem.dylib)","") endif endif -ifeq ($(STATIC_LUA),yes) -all: libvalkeylua.a - -libvalkeylua.a: $(OBJS) $(LIBS) - $(QUIET_LINK_MOD)$(AR) rcs $@ $(OBJS) -else +ifeq ($(BUILD_LUA),module) all: libvalkeylua.so libvalkeylua.so: $(OBJS) $(LIBS) $(QUIET_LINK_MOD)$(CC) -o $@ $(SHOBJ_LDFLAGS) $^ +else +all: libvalkeylua.a + +libvalkeylua.a: $(OBJS) $(LIBS) + $(QUIET_LINK_MOD)$(AR) rcs $@ $(OBJS) endif sha1.o: ../../sha1.c diff --git a/src/server.c b/src/server.c index 30eb98e6795..4e5374e49fc 100644 --- a/src/server.c +++ b/src/server.c @@ -7625,25 +7625,15 @@ __attribute__((weak)) int main(int argc, char **argv) { clusterInitLast(); } - /* Initialize the LUA scripting engine. */ -#ifdef LUA_ENABLED -#if STATIC_LUA +#if defined(LUA_ENABLED) && STATIC_LUA + /* Initialize the LUA scripting engine on-startup only when LUA is built statically */ if (scriptingEngineManagerFind("lua") == NULL) { if (moduleLoadStatic("lua", NULL, 0, 0) != C_OK) { serverPanic("Lua engine initialization failed, check the server logs."); } } -#else -#define LUA_LIB_STR STRINGIFY(LUA_LIB) - if (scriptingEngineManagerFind("lua") == NULL) { - if (moduleLoad(LUA_LIB_STR, NULL, 0, 0) != C_OK) { - serverPanic("Lua engine initialization failed, check the server logs."); - } - } -#endif #endif - InitServerLast(); if (!server.sentinel_mode) { From 2be8ea9b741923a8db452f0df7352811c50d735e Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Wed, 25 Mar 2026 22:00:32 +0200 Subject: [PATCH 05/18] Fix Lua Module Build Flag Propagation * Build system **Generated by CodeLite** Signed-off-by: Eran Ifrah --- src/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile b/src/Makefile index e68a4e58851..f74118bfa13 100644 --- a/src/Makefile +++ b/src/Makefile @@ -727,7 +727,7 @@ $(RDMA_MODULE_NAME): $(SERVER_NAME) # engine_lua.so $(LUA_MODULE_NAME): .make-prerequisites - $(MAKE) -C modules/lua OPTIMIZATION="$(OPTIMIZATION)" STATIC_LUA="$(STATIC_LUA)" + $(MAKE) -C modules/lua OPTIMIZATION="$(OPTIMIZATION)" BUILD_LUA="$(BUILD_LUA)" # valkey-cli $(ENGINE_CLI_NAME): $(ENGINE_CLI_OBJ) From 2010ad6164d43b8eacfe3ebd6cdc134465cecc31 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Sun, 29 Mar 2026 10:15:40 +0300 Subject: [PATCH 06/18] Fix Shutdown Hook Log Assertion Adjust the module API hook test to inspect a larger tail of the replica log when verifying the shutdown event. This makes the assertion less brittle and better aligned with the amount of output produced during shutdown. **Generated by CodeLite** Signed-off-by: Eran Ifrah --- tests/unit/moduleapi/hooks.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/moduleapi/hooks.tcl b/tests/unit/moduleapi/hooks.tcl index 241dabb277e..0050565db3e 100644 --- a/tests/unit/moduleapi/hooks.tcl +++ b/tests/unit/moduleapi/hooks.tcl @@ -373,7 +373,7 @@ tags "modules" { # look into the log file of the server that just exited test {Test shutdown hook} { - assert_equal [string match {*module-event-shutdown*} [exec tail -9 < $replica_stdout]] 1 + assert_equal [string match {*module-event-shutdown*} [exec tail -20 < $replica_stdout]] 1 } } From 4b7ff253142a2b7415ae5d7333ebda5bb257a12a Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Sun, 29 Mar 2026 16:29:07 +0300 Subject: [PATCH 07/18] Removed extra log line during shutdown The extra log line caused an error to some of the TCL tests. Signed-off-by: Eran Ifrah --- src/eval.c | 6 +----- tests/unit/moduleapi/hooks.tcl | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/eval.c b/src/eval.c index 1823cf35f11..876f376f6c9 100644 --- a/src/eval.c +++ b/src/eval.c @@ -190,9 +190,9 @@ void evalRelease(int async) { * Called when a scripting engine is unregistered to avoid dangling engine * pointers in the eval script cache. */ void evalRemoveScriptsOfEngine(scriptingEngine *engine, const char *engine_name) { + UNUSED(engine_name); dictIterator *iter = dictGetSafeIterator(evalCtx.scripts); dictEntry *entry; - size_t scripts_removed = 0; while ((entry = dictNext(iter))) { evalScript *es = dictGetVal(entry); if (es->engine == engine) { @@ -202,13 +202,9 @@ void evalRemoveScriptsOfEngine(scriptingEngine *engine, const char *engine_name) listDelNode(evalCtx.scripts_lru_list, es->node); } dictDelete(evalCtx.scripts, sha); - ++scripts_removed; } } dictReleaseIterator(iter); - serverLog(LL_NOTICE, - "Successfully removed %zu cached scripts associated with engine '%s'", - scripts_removed, engine_name); } void evalReset(int async) { diff --git a/tests/unit/moduleapi/hooks.tcl b/tests/unit/moduleapi/hooks.tcl index 0050565db3e..241dabb277e 100644 --- a/tests/unit/moduleapi/hooks.tcl +++ b/tests/unit/moduleapi/hooks.tcl @@ -373,7 +373,7 @@ tags "modules" { # look into the log file of the server that just exited test {Test shutdown hook} { - assert_equal [string match {*module-event-shutdown*} [exec tail -20 < $replica_stdout]] 1 + assert_equal [string match {*module-event-shutdown*} [exec tail -9 < $replica_stdout]] 1 } } From 412e8c89fca2e7099eb078ee87107023c5a9bb8c Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Mon, 30 Mar 2026 12:48:07 +0300 Subject: [PATCH 08/18] Use weak attributes on duplicate symbols In addition, removed duplicate code. Minor fix in the Makefile: settings LUA_MODULE_NAME to its correct name. Signed-off-by: Eran Ifrah --- src/Makefile | 4 +++- src/modules/lua/function_lua.c | 21 ++++----------------- src/modules/lua/script_lua.c | 15 +++++---------- 3 files changed, 12 insertions(+), 28 deletions(-) diff --git a/src/Makefile b/src/Makefile index f74118bfa13..9b2a611f895 100644 --- a/src/Makefile +++ b/src/Makefile @@ -254,12 +254,13 @@ endif FINAL_CFLAGS+= -I../deps/libvalkey/include -I../deps/linenoise -I../deps/hdr_histogram -I../deps/fpconv -I../deps/fast_float # Lua scripting engine module -LUA_MODULE_NAME:=modules/lua/libvalkeylua.so ifeq ($(BUILD_LUA),no) + LUA_MODULE_NAME= LUA_MODULE= LUA_MODULE_INSTALL= FINAL_CFLAGS+=-DSTATIC_LUA=0 else ifeq ($(BUILD_LUA),module) + LUA_MODULE_NAME=modules/lua/libvalkeylua.so LUA_MODULE=$(LUA_MODULE_NAME) LUA_MODULE_INSTALL=install-lua-module @@ -273,6 +274,7 @@ else ifeq ($(BUILD_LUA),module) endif else # The default: building Lua as a static module. + LUA_MODULE_NAME=modules/lua/libvalkeylua.a LUA_MODULE=$(LUA_MODULE_NAME) current_dir = $(shell pwd) FINAL_CFLAGS+=-DLUA_ENABLED -DSTATIC_LUA=1 diff --git a/src/modules/lua/function_lua.c b/src/modules/lua/function_lua.c index dbabd9a37d9..b9b68fb59d4 100644 --- a/src/modules/lua/function_lua.c +++ b/src/modules/lua/function_lua.c @@ -55,23 +55,10 @@ typedef uint64_t monotime; -static monotime getMonotonicUs(void) { - /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems - * did not support this until much later. CLOCK_MONOTONIC is technically - * optional and may not be supported - but it appears to be universal. - * If this is not supported, provide a system-specific alternate version. */ - struct timespec ts; - clock_gettime(CLOCK_MONOTONIC, &ts); - return ((uint64_t)ts.tv_sec) * 1000000 + ts.tv_nsec / 1000; -} - -static inline uint64_t elapsedUs(monotime start_time) { - return getMonotonicUs() - start_time; -} - -static inline uint64_t elapsedMs(monotime start_time) { - return elapsedUs(start_time) / 1000; -} +// Defined in script_lua.c +extern monotime getMonotonicUs(void); +extern monotime elapsedUs(monotime start_time); +extern monotime elapsedMs(monotime start_time); typedef struct loadCtx { List *functions; diff --git a/src/modules/lua/script_lua.c b/src/modules/lua/script_lua.c index f007a7d0229..d642b463754 100644 --- a/src/modules/lua/script_lua.c +++ b/src/modules/lua/script_lua.c @@ -172,11 +172,7 @@ static void _serverPanic(const char *file, int line, const char *msg, ...) { typedef uint64_t monotime; -#if STATIC_LUA -/* Use the engine's version */ -extern monotime getMonotonicUs(void); -#else -monotime getMonotonicUs(void) { +__attribute__((weak)) monotime getMonotonicUs(void) { /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems * did not support this until much later. CLOCK_MONOTONIC is technically * optional and may not be supported - but it appears to be universal. @@ -185,13 +181,12 @@ monotime getMonotonicUs(void) { clock_gettime(CLOCK_MONOTONIC, &ts); return ((uint64_t)ts.tv_sec) * 1000000 + ts.tv_nsec / 1000; } -#endif -inline uint64_t elapsedUs(monotime start_time) { +__attribute__((always_inline)) monotime elapsedUs(monotime start_time) { return getMonotonicUs() - start_time; } -inline uint64_t elapsedMs(monotime start_time) { +__attribute__((always_inline)) monotime elapsedMs(monotime start_time) { return elapsedUs(start_time) / 1000; } @@ -1167,7 +1162,7 @@ static int luaRedisPCallCommand(lua_State *lua) { * * 'digest' should point to a 41 bytes buffer: 40 for SHA1 converted into an * hexadecimal number, plus 1 byte for null term. */ -void sha1Hex(char *digest, char *script, size_t len) { +__attribute__((weak)) void sha1hex(char *digest, char *script, size_t len) { SHA1_CTX ctx; unsigned char hash[20]; char *cset = "0123456789abcdef"; @@ -1198,7 +1193,7 @@ static int luaRedisSha1hexCommand(lua_State *lua) { } s = (char *)lua_tolstring(lua, 1, &len); - sha1Hex(digest, s, len); + sha1hex(digest, s, len); lua_pushstring(lua, digest); return 1; } From 08553b6f199b0ebc690c233101607c8d8f4628a1 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Mon, 30 Mar 2026 15:28:28 +0300 Subject: [PATCH 09/18] Mark `sha1hex` as a weak symbol Signed-off-by: Eran Ifrah --- src/modules/lua/function_lua.c | 21 +++++++++++++++++---- src/modules/lua/script_lua.c | 13 +++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/modules/lua/function_lua.c b/src/modules/lua/function_lua.c index b9b68fb59d4..dbabd9a37d9 100644 --- a/src/modules/lua/function_lua.c +++ b/src/modules/lua/function_lua.c @@ -55,10 +55,23 @@ typedef uint64_t monotime; -// Defined in script_lua.c -extern monotime getMonotonicUs(void); -extern monotime elapsedUs(monotime start_time); -extern monotime elapsedMs(monotime start_time); +static monotime getMonotonicUs(void) { + /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems + * did not support this until much later. CLOCK_MONOTONIC is technically + * optional and may not be supported - but it appears to be universal. + * If this is not supported, provide a system-specific alternate version. */ + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + return ((uint64_t)ts.tv_sec) * 1000000 + ts.tv_nsec / 1000; +} + +static inline uint64_t elapsedUs(monotime start_time) { + return getMonotonicUs() - start_time; +} + +static inline uint64_t elapsedMs(monotime start_time) { + return elapsedUs(start_time) / 1000; +} typedef struct loadCtx { List *functions; diff --git a/src/modules/lua/script_lua.c b/src/modules/lua/script_lua.c index d642b463754..9dabde1afd5 100644 --- a/src/modules/lua/script_lua.c +++ b/src/modules/lua/script_lua.c @@ -172,7 +172,11 @@ static void _serverPanic(const char *file, int line, const char *msg, ...) { typedef uint64_t monotime; -__attribute__((weak)) monotime getMonotonicUs(void) { +#if STATIC_LUA +/* Use the engine's version */ +extern monotime getMonotonicUs(void); +#else +monotime getMonotonicUs(void) { /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems * did not support this until much later. CLOCK_MONOTONIC is technically * optional and may not be supported - but it appears to be universal. @@ -181,12 +185,13 @@ __attribute__((weak)) monotime getMonotonicUs(void) { clock_gettime(CLOCK_MONOTONIC, &ts); return ((uint64_t)ts.tv_sec) * 1000000 + ts.tv_nsec / 1000; } +#endif -__attribute__((always_inline)) monotime elapsedUs(monotime start_time) { +inline uint64_t elapsedUs(monotime start_time) { return getMonotonicUs() - start_time; } -__attribute__((always_inline)) monotime elapsedMs(monotime start_time) { +inline uint64_t elapsedMs(monotime start_time) { return elapsedUs(start_time) / 1000; } @@ -1162,7 +1167,7 @@ static int luaRedisPCallCommand(lua_State *lua) { * * 'digest' should point to a 41 bytes buffer: 40 for SHA1 converted into an * hexadecimal number, plus 1 byte for null term. */ -__attribute__((weak)) void sha1hex(char *digest, char *script, size_t len) { +__attribute((weak)) void sha1hex(char *digest, char *script, size_t len) { SHA1_CTX ctx; unsigned char hash[20]; char *cset = "0123456789abcdef"; From ec69dd1a56545319e48080f441f6584fc7aab078 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Tue, 7 Apr 2026 08:19:58 +0000 Subject: [PATCH 10/18] Fixed loading on FreeBSD Signed-off-by: Eran Ifrah --- src/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Makefile b/src/Makefile index d78ec688277..849c0d3fc04 100644 --- a/src/Makefile +++ b/src/Makefile @@ -280,6 +280,8 @@ else FINAL_CFLAGS+=-DLUA_ENABLED -DSTATIC_LUA=1 ifeq ($(uname_S),Darwin) LUA_LDFLAGS=-Wl,-export_dynamic -Wl,-force_load,$(current_dir)/modules/lua/libvalkeylua.a ../deps/lua/src/liblua.a + else ifeq ($(uname_S),FreeBSD) + LUA_LDFLAGS=-Wl,--export-dynamic -Wl,--whole-archive $(current_dir)/modules/lua/libvalkeylua.a -Wl,--no-whole-archive ../deps/lua/src/liblua.a else LUA_LDFLAGS=-Wl,--whole-archive $(current_dir)/modules/lua/libvalkeylua.a -Wl,--no-whole-archive ../deps/lua/src/liblua.a endif From 686fde1e569bf3d4e7a9a77286f69a7bf15e5989 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Tue, 7 Apr 2026 11:57:19 +0300 Subject: [PATCH 11/18] Refactor Static Module Loading Documentation Update the documentation comments for the static module loading helpers to better match Valkey's standard. * API docs **Generated by CodeLite** Signed-off-by: Eran Ifrah --- src/module.c | 65 ++++++++++++++++------------------------------------ 1 file changed, 20 insertions(+), 45 deletions(-) diff --git a/src/module.c b/src/module.c index f121c77a8ca..e01f4f47d1b 100644 --- a/src/module.c +++ b/src/module.c @@ -12919,30 +12919,11 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa return C_OK; } -/** - * Load a static symbol from the current process by constructing a module-specific symbol name. - * - * This function builds the lookup name as "_", opens the current - * process image with dlopen(), and then resolves the symbol with dlsym(). On failure, it logs - * a warning and returns C_ERR; on success, it stores the resolved address and handle and returns - * C_OK. - * - * @param out - * void **. Output pointer that receives the address of the resolved symbol. - * @param handle - * void **. Output pointer that receives the dlopen() handle for the current process. - * @param symbol_name - * const char *. Base name of the symbol to resolve. - * @param module_name - * const char *. Module name appended to the symbol name to form the final lookup name. - * - * @return int - * C_OK on success, or C_ERR if the symbol name is too long, dlopen() fails, or dlsym() - * cannot resolve the symbol. - * - * @note This function is intended for use in the module-loading context and relies on the - * current process exposing statically linked module entry points under the constructed name. - */ +/* Resolve a symbol from a statically linked module. The symbol is looked up + * by constructing the name "_" and searching for it + * in the current process via dlopen(NULL)/dlsym(). On success, '*out' is set + * to the symbol address, '*handle' is set to the dlopen handle, and C_OK is + * returned. On failure C_ERR is returned and an appropriate warning is logged. */ static int moduleLoadStaticSymbol(void **out, void **handle, const char *symbol_name, const char *module_name) { char symbol_full_name[128]; int n = snprintf(symbol_full_name, sizeof(symbol_full_name), "%s_%s", symbol_name, module_name); @@ -12975,31 +12956,25 @@ static int moduleLoadStaticSymbol(void **out, void **handle, const char *symbol_ return C_OK; } -/** - * @brief Loads a statically linked module by locating its on-load and on-unload entry points. +/* Load a statically linked module and initialize it. This is the static + * counterpart of moduleLoad(): instead of dlopen()ing a shared object from + * a file path, it resolves the module's entry point from the running + * executable itself. * - * This function builds the expected symbol names for the module, resolves them from the current - * process image, and invokes the module's initialization routine. On success, the module is registered - * with the server, its state is initialized, and module load events are fired. If validation or - * post-load checks fail, the module is unloaded and an error is returned. + * The entry point is located by constructing the symbol name + * "ValkeyModule_OnLoad_" and resolving it with + * moduleLoadStaticSymbol(). For example, a module named "mymodule" must + * provide a function called ValkeyModule_OnLoad_mymodule. * - * @param module_name const char * Name of the static module, used to construct the on-load and - * on-unload symbol names. - * @param module_argv void ** Array of module arguments passed through to the module's on-load - * function. - * @param module_argc int Number of entries in module_argv. - * @param is_loadex int Nonzero if the module is being loaded via the loadex path and queued - * configuration arguments should be validated. + * Once the entry point is found, the function creates a temporary module + * context, invokes the OnLoad callback, and on success registers the module + * in the global modules dictionary with is_static_module set to 1 and handle + * set to NULL (since there is no shared-object handle to keep open). * - * @return int C_OK on successful load; C_ERR if symbol lookup fails, module initialization fails, - * or post-load validation detects an error. + * If 'is_loadex' is true, the function also validates that all queued module + * configurations were consumed; otherwise the module is unloaded. * - * @note This function operates in the server/module loading context and expects the module's - * ValkeyModule_OnLoad_ and ValkeyModule_OnUnload_ symbols to be present in the - * current process. - * - * @throws No exceptions are thrown. Errors are reported via server logging and by returning C_ERR. - */ + * On success C_OK is returned, otherwise C_ERR is returned. */ int moduleLoadStatic(const char *module_name, void **module_argv, int module_argc, int is_loadex) { // Locate the load ModuleLoadFunc onload; From 6e25514c5008614d2042746b4f71357d9ff97507 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Tue, 7 Apr 2026 12:05:11 +0300 Subject: [PATCH 12/18] Lua Makefile: Use a Variable to Declare the Target Signed-off-by: Eran Ifrah --- src/modules/lua/Makefile | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/modules/lua/Makefile b/src/modules/lua/Makefile index 4a486a4b3c4..8f534a51b48 100644 --- a/src/modules/lua/Makefile +++ b/src/modules/lua/Makefile @@ -5,8 +5,10 @@ DEPS_DIR=../../../deps ifeq ($(BUILD_LUA),module) STATIC_LUA_FLAG=-DSTATIC_LUA=0 + LUA_TARGET=libvalkeylua.so else STATIC_LUA_FLAG=-DSTATIC_LUA=1 + LUA_TARGET=libvalkeylua.a endif ifeq ($(uname_S),Darwin) @@ -66,15 +68,12 @@ ifeq ("$(wildcard /usr/lib/libSystem.dylib)","") endif endif +all: $(LUA_TARGET) ifeq ($(BUILD_LUA),module) -all: libvalkeylua.so - -libvalkeylua.so: $(OBJS) $(LIBS) +$(LUA_TARGET): $(OBJS) $(LIBS) $(QUIET_LINK_MOD)$(CC) -o $@ $(SHOBJ_LDFLAGS) $^ else -all: libvalkeylua.a - -libvalkeylua.a: $(OBJS) $(LIBS) +$(LUA_TARGET): $(OBJS) $(LIBS) $(QUIET_LINK_MOD)$(AR) rcs $@ $(OBJS) endif From c1ad9c5746d214e825bc4865dd63137f192de737 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Sun, 12 Apr 2026 13:09:25 +0300 Subject: [PATCH 13/18] Removed un-needed attributes - Removed __attribute__((visibility("default"))) - Added comments for the LUA_MODULE_VISIBILITY macro. **Generated by CodeLite** Signed-off-by: Eran Ifrah --- src/modules/lua/engine_lua.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/modules/lua/engine_lua.c b/src/modules/lua/engine_lua.c index 5141ef9561e..4013d80e0a2 100644 --- a/src/modules/lua/engine_lua.c +++ b/src/modules/lua/engine_lua.c @@ -481,6 +481,11 @@ static void luaEngineDebuggerEnd(ValkeyModuleCtx *module_ctx, static struct luaEngineCtx *engine_ctx = NULL; #if STATIC_LUA +/* + * When building Lua as a static library, hide the generic module entry + * points, ValkeyModule_OnLoad and ValkeyModule_OnLoad, to avoid multiple + * symbol definitions. This is done by declaring these functions as static. + */ #define LUA_MODULE_VISIBILITY static #else #define LUA_MODULE_VISIBILITY @@ -552,12 +557,14 @@ LUA_MODULE_VISIBILITY int ValkeyModule_OnUnload(ValkeyModuleCtx *ctx) { return VALKEYMODULE_OK; } -__attribute__((visibility("default"))) int ValkeyModule_OnLoad_lua(ValkeyModuleCtx *ctx, - ValkeyModuleString **argv, - int argc) { +/* Unique entry points (Load and Unload) used by the Lua module when linked statically */ + +int ValkeyModule_OnLoad_lua(ValkeyModuleCtx *ctx, + ValkeyModuleString **argv, + int argc) { return ValkeyModule_OnLoad(ctx, argv, argc); } -__attribute__((visibility("default"))) int ValkeyModule_OnUnload_lua(ValkeyModuleCtx *ctx) { +int ValkeyModule_OnUnload_lua(ValkeyModuleCtx *ctx) { return ValkeyModule_OnUnload(ctx); } From cf5a994dfddeca5d8d052229e486c4b3f29b1a73 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Mon, 13 Apr 2026 17:41:27 +0300 Subject: [PATCH 14/18] Refine module lifecycle and cleanup logic The module system and Lua integration are updated to enhance reliability and stability, particularly during module loading and unloading. Key changes include: * Implementing conditional compilation for Lua module entry points to support static linking gracefully. * Hardening module unloading logic in `module.c` by adding checks before calling `dlclose` and improving error logging if the unload function fails to load. * Removing the redundant `freeCachedScriptsForEngine` function in the evaluation subsystem. * use `__attribute__((weak))` instead of `__attribute((weak))` in `src/modules/lua/script_lua.c`. * Module Management * Lua Integration * Script Evaluation **Generated by CodeLite** Signed-off-by: Eran Ifrah --- src/eval.c | 6 ------ src/module.c | 7 ++++--- src/modules/lua/engine_lua.c | 3 ++- src/modules/lua/script_lua.c | 2 +- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/eval.c b/src/eval.c index 4b0496b289d..92c14ace495 100644 --- a/src/eval.c +++ b/src/eval.c @@ -158,12 +158,6 @@ void freeEvalScripts(dict *scripts, list *scripts_lru_list, list *engine_callbac } } -void freeCachedScriptsForEngine(scriptingEngine *engine) { - if (engine == NULL) { - return; - } -} - static void resetEngineEvalEnvCallback(scriptingEngine *engine, void *context) { int async = context != NULL; callableLazyEnvReset *callback = scriptingEngineCallResetEnvFunc(engine, VMSE_EVAL, async); diff --git a/src/module.c b/src/module.c index fe502422d2d..c0d4902df38 100644 --- a/src/module.c +++ b/src/module.c @@ -13204,7 +13204,7 @@ static int moduleLoadStaticSymbol(void **out, void **handle, const char *symbol_ return C_ERR; } - // Open a handle to self + /* Open a handle to self */ *handle = dlopen(NULL, RTLD_NOW); if (*handle == NULL) { char *error = dlerror(); @@ -13248,7 +13248,6 @@ static int moduleLoadStaticSymbol(void **out, void **handle, const char *symbol_ * * On success C_OK is returned, otherwise C_ERR is returned. */ int moduleLoadStatic(const char *module_name, void **module_argv, int module_argc, int is_loadex) { - // Locate the load ModuleLoadFunc onload; void *handle = NULL; if (moduleLoadStaticSymbol((void **)&onload, &handle, "ValkeyModule_OnLoad", module_name) != C_OK) { @@ -13357,6 +13356,8 @@ static int moduleUnloadInternal(struct ValkeyModule *module, const char **errmsg ModuleUnLoadFunc onunload = NULL; if (module->is_static_module == 1) { if (moduleLoadStaticSymbol((void **)&onunload, &module->handle, "ValkeyModule_OnUnload", module->name) != C_OK) { + serverLog(LL_WARNING, "Module %s OnUnload failed. Unload canceled.", module->name); + errno = ECANCELED; return C_ERR; } } else { @@ -13388,7 +13389,7 @@ static int moduleUnloadInternal(struct ValkeyModule *module, const char **errmsg moduleUnregisterCleanup(module); /* Unload the dynamic library. */ - if (dlclose(module->handle) == -1) { + if (module->handle != NULL && dlclose(module->handle) == -1) { char *error = dlerror(); if (error == NULL) error = "Unknown error"; serverLog(LL_WARNING, "Error when trying to close the %s module: %s", module->name, error); diff --git a/src/modules/lua/engine_lua.c b/src/modules/lua/engine_lua.c index 4013d80e0a2..91582481abc 100644 --- a/src/modules/lua/engine_lua.c +++ b/src/modules/lua/engine_lua.c @@ -557,8 +557,8 @@ LUA_MODULE_VISIBILITY int ValkeyModule_OnUnload(ValkeyModuleCtx *ctx) { return VALKEYMODULE_OK; } +#if STATIC_LUA /* Unique entry points (Load and Unload) used by the Lua module when linked statically */ - int ValkeyModule_OnLoad_lua(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) { @@ -568,3 +568,4 @@ int ValkeyModule_OnLoad_lua(ValkeyModuleCtx *ctx, int ValkeyModule_OnUnload_lua(ValkeyModuleCtx *ctx) { return ValkeyModule_OnUnload(ctx); } +#endif diff --git a/src/modules/lua/script_lua.c b/src/modules/lua/script_lua.c index 0e7fd98acaf..4b5684d2009 100644 --- a/src/modules/lua/script_lua.c +++ b/src/modules/lua/script_lua.c @@ -1285,7 +1285,7 @@ static int luaRedisPCallCommand(lua_State *lua) { * * 'digest' should point to a 41 bytes buffer: 40 for SHA1 converted into an * hexadecimal number, plus 1 byte for null term. */ -__attribute((weak)) void sha1hex(char *digest, char *script, size_t len) { +__attribute__((weak)) void sha1hex(char *digest, char *script, size_t len) { SHA1_CTX ctx; unsigned char hash[20]; char *cset = "0123456789abcdef"; From 83c258f11c1db74318ee357193a3cbf96dc226ff Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Mon, 13 Apr 2026 19:57:53 +0300 Subject: [PATCH 15/18] Remove always_inline attribute from `getClientType` Signed-off-by: Eran Ifrah --- src/server.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.h b/src/server.h index bdf75080747..b4d5021bb20 100644 --- a/src/server.h +++ b/src/server.h @@ -1422,7 +1422,7 @@ bool isImportSlotMigrationJob(slotMigrationJob *job); * CLIENT_TYPE_PUBSUB -> Client subscribed to Pub/Sub channels * CLIENT_TYPE_PRIMARY -> The client representing our replication primary. */ -static inline __attribute__((always_inline)) int getClientType(client *c) { +static inline int getClientType(client *c) { if (unlikely(c->flag.primary)) return CLIENT_TYPE_PRIMARY; /* Even though MONITOR clients are marked as replicas, we * want the expose them as normal clients. */ From f0886dba5d00532a1a47f053d3758b58724ce233 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Tue, 14 Apr 2026 14:59:06 +0300 Subject: [PATCH 16/18] Refactor Module Loading to Eliminate Code Duplication Extract the common module initialization logic from `moduleLoad` and `moduleLoadStatic` into a new helper function `moduleInitPostOnLoadResolved`. This function handles the shared workflow of invoking the onload callback, registering the module, performing post-load validation, and firing server events. The refactoring eliminates approximately 70 lines of duplicated code while preserving all existing behavior. The helper function accepts parameters that control static vs. dynamic module handling, including whether to close the dlopen handle and how to format log messages. Both `moduleLoad` and `moduleLoadStatic` now focus on their specific responsibilities (dlopen/symbol resolution) and delegate the common initialization sequence to the shared helper. * Module loading infrastructure * Code deduplication **Generated by CodeLite** Signed-off-by: Eran Ifrah --- src/module.c | 211 +++++++++++++++++++++------------------------------ 1 file changed, 87 insertions(+), 124 deletions(-) diff --git a/src/module.c b/src/module.c index c0d4902df38..79d20e0ffb0 100644 --- a/src/module.c +++ b/src/module.c @@ -13074,82 +13074,52 @@ void moduleUnregisterCleanup(ValkeyModule *module) { moduleUnregisterAuthCBs(module); } -/* Load a module and initialize it. On success C_OK is returned, otherwise - * C_ERR is returned. */ -int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loadex) { - int (*onload)(void *, void **, int); - void *handle; - - struct stat st; - if (stat(path, &st) == 0) { - /* This check is best effort */ - if (!(st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH))) { - serverLog(LL_WARNING, "Module %s failed to load: It does not have execute permissions.", path); - return C_ERR; - } - } - - int dlopen_flags = RTLD_NOW | RTLD_LOCAL; -#if (defined(__GLIBC__) || defined(__FreeBSD__)) && !defined(VALKEY_ADDRESS_SANITIZER) && !defined(VALKEY_THREAD_SANITIZER) && __has_include() - /* RTLD_DEEPBIND, which is required for loading modules that contains the - * same symbols, does not work with ASAN or TSAN. Therefore, we exclude - * RTLD_DEEPBIND when doing test builds with sanitizers. - * See https://github.com/google/sanitizers/issues/611 for more details. - * - * This flag is also currently only available in Linux and FreeBSD. */ - dlopen_flags |= RTLD_DEEPBIND; -#endif - - handle = dlopen(path, dlopen_flags); - if (handle == NULL) { - serverLog(LL_WARNING, "Module %s failed to load: %s", path, dlerror()); - return C_ERR; - } - - const char *onLoadNames[] = {"ValkeyModule_OnLoad", "RedisModule_OnLoad"}; - for (size_t i = 0; i < sizeof(onLoadNames) / sizeof(onLoadNames[0]); i++) { - onload = (int (*)(void *, void **, int))(unsigned long)dlsym(handle, onLoadNames[i]); - if (onload != NULL) { - if (i != 0) { - serverLog(LL_NOTICE, "Legacy Redis Module %s found", path); - } - break; - } - } - - if (onload == NULL) { - dlclose(handle); - serverLog(LL_WARNING, - "Module %s does not export ValkeyModule_OnLoad() or RedisModule_OnLoad() " - "symbol. Module not loaded.", - path); - return C_ERR; - } +/* Common helper for moduleLoad and moduleLoadStatic. + * Invokes the onload callback, registers the module, and performs post-load + * validation. 'display_name' is used in log messages, 'handle' is the + * dlopen handle (NULL for static modules), and 'is_static' controls the + * is_static_module flag and handle ownership semantics. */ +static int moduleInitPostOnLoadResolved(ModuleLoadFunc onload, + void *handle, + const char *display_name, + void **module_argv, + int module_argc, + int is_loadex, + int is_static) { ValkeyModuleCtx ctx; moduleCreateContext(&ctx, NULL, VALKEYMODULE_CTX_TEMP_CLIENT); /* We pass NULL since we don't have a module yet. */ if (onload((void *)&ctx, module_argv, module_argc) == VALKEYMODULE_ERR) { if (ctx.module) { - serverLog(LL_WARNING, "Module %s initialization failed. Module not loaded.", path); + serverLog(LL_WARNING, "%sModule %s initialization failed. Module not loaded.", + is_static ? "Static " : "", display_name); moduleUnregisterCleanup(ctx.module); moduleRemoveCateogires(ctx.module); moduleFreeModuleStructure(ctx.module); } else { /* If there is no ctx.module, this means that our ValkeyModule_Init call failed, * and currently init will only fail on busy name. */ - serverLog(LL_WARNING, "Module %s initialization failed. Module name is busy.", path); + serverLog(LL_WARNING, "%sModule %s initialization failed. Module name is busy.", + is_static ? "Static " : "", display_name); } moduleFreeContext(&ctx); - dlclose(handle); + if (handle) { + dlclose(handle); + } return C_ERR; } + if (is_static && handle) { + dlclose(handle); + handle = NULL; + } + /* Module loaded! Register it. */ dictAdd(modules, ctx.module->name, ctx.module); ctx.module->blocked_clients = 0; ctx.module->handle = handle; - ctx.module->is_static_module = 0; + ctx.module->is_static_module = is_static; ctx.module->loadmod = zmalloc(sizeof(struct moduleLoadQueueEntry)); - ctx.module->loadmod->path = sdsnew(path); + ctx.module->loadmod->path = sdsnew(display_name); ctx.module->loadmod->argv = module_argc ? zmalloc(sizeof(robj *) * module_argc) : NULL; ctx.module->loadmod->argc = module_argc; for (int i = 0; i < module_argc; i++) { @@ -13162,7 +13132,11 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa if (ctx.module->num_commands_with_acl_categories) { ACLRecomputeCommandBitsFromCommandRulesAllUsers(); } - serverLog(LL_NOTICE, "Module '%s' loaded from %s", ctx.module->name, path); + if (is_static) { + serverLog(LL_NOTICE, "Static Module '%s' successfully loaded", ctx.module->name); + } else { + serverLog(LL_NOTICE, "Module '%s' loaded from %s", ctx.module->name, display_name); + } ctx.module->onload = 0; int post_load_err = 0; @@ -13186,11 +13160,64 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa /* Fire the loaded modules event. */ moduleFireServerEvent(VALKEYMODULE_EVENT_MODULE_CHANGE, VALKEYMODULE_SUBEVENT_MODULE_LOADED, ctx.module); - moduleFreeContext(&ctx); return C_OK; } +/* Load a module and initialize it. On success C_OK is returned, otherwise + * C_ERR is returned. */ +int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loadex) { + ModuleLoadFunc onload; + void *handle; + + struct stat st; + if (stat(path, &st) == 0) { + /* This check is best effort */ + if (!(st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH))) { + serverLog(LL_WARNING, "Module %s failed to load: It does not have execute permissions.", path); + return C_ERR; + } + } + + int dlopen_flags = RTLD_NOW | RTLD_LOCAL; +#if (defined(__GLIBC__) || defined(__FreeBSD__)) && !defined(VALKEY_ADDRESS_SANITIZER) && !defined(VALKEY_THREAD_SANITIZER) && __has_include() + /* RTLD_DEEPBIND, which is required for loading modules that contains the + * same symbols, does not work with ASAN or TSAN. Therefore, we exclude + * RTLD_DEEPBIND when doing test builds with sanitizers. + * See https://github.com/google/sanitizers/issues/611 for more details. + * + * This flag is also currently only available in Linux and FreeBSD. */ + dlopen_flags |= RTLD_DEEPBIND; +#endif + + handle = dlopen(path, dlopen_flags); + if (handle == NULL) { + serverLog(LL_WARNING, "Module %s failed to load: %s", path, dlerror()); + return C_ERR; + } + + const char *onLoadNames[] = {"ValkeyModule_OnLoad", "RedisModule_OnLoad"}; + for (size_t i = 0; i < sizeof(onLoadNames) / sizeof(onLoadNames[0]); i++) { + onload = (ModuleLoadFunc)(unsigned long)dlsym(handle, onLoadNames[i]); + if (onload != NULL) { + if (i != 0) { + serverLog(LL_NOTICE, "Legacy Redis Module %s found", path); + } + break; + } + } + + if (onload == NULL) { + dlclose(handle); + serverLog(LL_WARNING, + "Module %s does not export ValkeyModule_OnLoad() or RedisModule_OnLoad() " + "symbol. Module not loaded.", + path); + return C_ERR; + } + return moduleInitPostOnLoadResolved(onload, handle, path, module_argv, module_argc, is_loadex, 0); +} + /* Resolve a symbol from a statically linked module. The symbol is looked up * by constructing the name "_" and searching for it * in the current process via dlopen(NULL)/dlsym(). On success, '*out' is set @@ -13253,72 +13280,8 @@ int moduleLoadStatic(const char *module_name, void **module_argv, int module_arg if (moduleLoadStaticSymbol((void **)&onload, &handle, "ValkeyModule_OnLoad", module_name) != C_OK) { return C_ERR; } - - ValkeyModuleCtx ctx; - moduleCreateContext(&ctx, NULL, VALKEYMODULE_CTX_TEMP_CLIENT); /* We pass NULL since we don't have a module yet. */ - if (onload((void *)&ctx, module_argv, module_argc) == VALKEYMODULE_ERR) { - if (ctx.module) { - serverLog(LL_WARNING, "Static Module %s initialization failed. Module not loaded.", module_name); - moduleUnregisterCleanup(ctx.module); - moduleRemoveCateogires(ctx.module); - moduleFreeModuleStructure(ctx.module); - } else { - /* If there is no ctx.module, this means that our ValkeyModule_Init call failed, - * and currently init will only fail on busy name. */ - serverLog(LL_WARNING, "Static Module %s initialization failed. Module name is busy.", module_name); - } - moduleFreeContext(&ctx); - dlclose(handle); - return C_ERR; - } - - dlclose(handle); - - /* Module loaded! Register it. */ - dictAdd(modules, ctx.module->name, ctx.module); - ctx.module->blocked_clients = 0; - ctx.module->handle = NULL; // We will re-open when needed. - ctx.module->is_static_module = 1; - ctx.module->loadmod = zmalloc(sizeof(struct moduleLoadQueueEntry)); - ctx.module->loadmod->path = sdsnew(module_name); - ctx.module->loadmod->argv = module_argc ? zmalloc(sizeof(robj *) * module_argc) : NULL; - ctx.module->loadmod->argc = module_argc; - for (int i = 0; i < module_argc; i++) { - ctx.module->loadmod->argv[i] = module_argv[i]; - incrRefCount(ctx.module->loadmod->argv[i]); - } - - /* If module commands have ACL categories, recompute command bits - * for all existing users once the modules has been registered. */ - if (ctx.module->num_commands_with_acl_categories) { - ACLRecomputeCommandBitsFromCommandRulesAllUsers(); - } - serverLog(LL_NOTICE, "Static Module '%s' successfully loaded", ctx.module->name); - ctx.module->onload = 0; - - int post_load_err = 0; - if (listLength(ctx.module->module_configs) && !ctx.module->configs_initialized) { - serverLogRaw(LL_WARNING, - "Module Configurations were not set, likely a missing LoadConfigs call. Unloading the module."); - post_load_err = 1; - } - - if (is_loadex && dictSize(server.module_configs_queue)) { - serverLogRaw(LL_WARNING, - "Loadex configurations were not applied, likely due to invalid arguments. Unloading the module."); - post_load_err = 1; - } - - if (post_load_err) { - moduleUnload(ctx.module->name, NULL); - moduleFreeContext(&ctx); - return C_ERR; - } - - /* Fire the loaded modules event. */ - moduleFireServerEvent(VALKEYMODULE_EVENT_MODULE_CHANGE, VALKEYMODULE_SUBEVENT_MODULE_LOADED, ctx.module); - moduleFreeContext(&ctx); - return C_OK; + return moduleInitPostOnLoadResolved(onload, handle, module_name, module_argv, module_argc, + is_loadex, 1); } static int moduleUnloadInternal(struct ValkeyModule *module, const char **errmsg) { From 251f2fe6e8a14750a2164075d2e425191e34867f Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Tue, 14 Apr 2026 15:32:52 +0300 Subject: [PATCH 17/18] Addressed comments - Move `evalRemoveScriptsOfEngine` code to a separate PR - Renamed `lua_scripts_flags_def` -> `lua_scripts_flags` with `static` Signed-off-by: Eran Ifrah --- src/eval.c | 22 ---------------------- src/eval.h | 3 --- src/modules/lua/function_lua.c | 4 ++-- src/scripting_engine.c | 2 -- 4 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/eval.c b/src/eval.c index 92c14ace495..ea2bb98c133 100644 --- a/src/eval.c +++ b/src/eval.c @@ -181,28 +181,6 @@ void evalRelease(int async) { } } - -/* Remove all cached eval scripts associated with the given scripting engine. - * Called when a scripting engine is unregistered to avoid dangling engine - * pointers in the eval script cache. */ -void evalRemoveScriptsOfEngine(scriptingEngine *engine, const char *engine_name) { - UNUSED(engine_name); - dictIterator *iter = dictGetSafeIterator(evalCtx.scripts); - dictEntry *entry; - while ((entry = dictNext(iter))) { - evalScript *es = dictGetVal(entry); - if (es->engine == engine) { - sds sha = dictGetKey(entry); - evalCtx.scripts_mem -= sdsAllocSize(sha) + getStringObjectSdsUsedMemory(es->body); - if (es->node) { - listDelNode(evalCtx.scripts_lru_list, es->node); - } - dictDelete(evalCtx.scripts, sha); - } - } - dictReleaseIterator(iter); -} - void evalReset(int async) { evalRelease(async); evalInit(); diff --git a/src/eval.h b/src/eval.h index 280071ad4be..f9c217ef0da 100644 --- a/src/eval.h +++ b/src/eval.h @@ -1,11 +1,8 @@ #ifndef _EVAL_H_ #define _EVAL_H_ -typedef struct scriptingEngine scriptingEngine; - void evalInit(void); void evalReset(int async); -void evalRemoveScriptsOfEngine(scriptingEngine *engine, const char *engine_name); void *evalActiveDefragScript(void *ptr); #endif /* _EVAL_H_ */ diff --git a/src/modules/lua/function_lua.c b/src/modules/lua/function_lua.c index dbabd9a37d9..7094636b71d 100644 --- a/src/modules/lua/function_lua.c +++ b/src/modules/lua/function_lua.c @@ -213,7 +213,7 @@ typedef struct flagStr { const char *str; } flagStr; -flagStr lua_scripts_flags_def[] = { +static flagStr lua_scripts_flags[] = { {.flag = VMSE_SCRIPT_FLAG_NO_WRITES, .str = "no-writes"}, {.flag = VMSE_SCRIPT_FLAG_ALLOW_OOM, .str = "allow-oom"}, {.flag = VMSE_SCRIPT_FLAG_ALLOW_STALE, .str = "allow-stale"}, @@ -244,7 +244,7 @@ static int luaRegisterFunctionReadFlags(lua_State *lua, uint64_t *flags) { const char *flag_str = lua_tostring(lua, -1); int found = 0; - for (flagStr *flag = lua_scripts_flags_def; flag->str; ++flag) { + for (flagStr *flag = lua_scripts_flags; flag->str; ++flag) { if (!strcasecmp(flag->str, flag_str)) { f_flags |= flag->flag; found = 1; diff --git a/src/scripting_engine.c b/src/scripting_engine.c index 9600a487b04..750b9564b08 100644 --- a/src/scripting_engine.c +++ b/src/scripting_engine.c @@ -7,7 +7,6 @@ #include "scripting_engine.h" #include "bio.h" #include "dict.h" -#include "eval.h" #include "functions.h" #include "module.h" #include "server.h" @@ -179,7 +178,6 @@ int scriptingEngineManagerUnregister(const char *engine_name) { scriptingEngine *e = dictGetVal(entry); functionsRemoveLibFromEngine(e); - evalRemoveScriptsOfEngine(e, e->name); engineMemoryInfo mem_info = scriptingEngineCallGetMemoryInfo(e, VMSE_ALL); engineMgr.total_memory_overhead -= zmalloc_size(e) + From 17c01284c1c1e4c0aff418b20495978cc924c7c2 Mon Sep 17 00:00:00 2001 From: Eran Ifrah Date: Tue, 14 Apr 2026 15:59:17 +0300 Subject: [PATCH 18/18] When possible, use engine's time helpers When building static lua, use: - `getMonotonicUs()` - `elapsedUs` - `elapsedMs` from the engine's `monotonic.h` header. Signed-off-by: Eran Ifrah --- src/modules/lua/function_lua.c | 12 ++++++++---- src/modules/lua/script_lua.c | 25 ------------------------- 2 files changed, 8 insertions(+), 29 deletions(-) diff --git a/src/modules/lua/function_lua.c b/src/modules/lua/function_lua.c index 7094636b71d..95283644a45 100644 --- a/src/modules/lua/function_lua.c +++ b/src/modules/lua/function_lua.c @@ -55,7 +55,11 @@ typedef uint64_t monotime; -static monotime getMonotonicUs(void) { +#if STATIC_LUA +/* Use the engine's version */ +#include "../../monotonic.h" +#else +monotime getMonotonicUs(void) { /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems * did not support this until much later. CLOCK_MONOTONIC is technically * optional and may not be supported - but it appears to be universal. @@ -64,14 +68,14 @@ static monotime getMonotonicUs(void) { clock_gettime(CLOCK_MONOTONIC, &ts); return ((uint64_t)ts.tv_sec) * 1000000 + ts.tv_nsec / 1000; } - -static inline uint64_t elapsedUs(monotime start_time) { +inline uint64_t elapsedUs(monotime start_time) { return getMonotonicUs() - start_time; } -static inline uint64_t elapsedMs(monotime start_time) { +inline uint64_t elapsedMs(monotime start_time) { return elapsedUs(start_time) / 1000; } +#endif typedef struct loadCtx { List *functions; diff --git a/src/modules/lua/script_lua.c b/src/modules/lua/script_lua.c index 4b5684d2009..5786a3fbe44 100644 --- a/src/modules/lua/script_lua.c +++ b/src/modules/lua/script_lua.c @@ -176,31 +176,6 @@ static void _serverPanic(const char *file, int line, const char *msg, ...) { #define serverPanic(...) _serverPanic(__FILE__, __LINE__, __VA_ARGS__) -typedef uint64_t monotime; - -#if STATIC_LUA -/* Use the engine's version */ -extern monotime getMonotonicUs(void); -#else -monotime getMonotonicUs(void) { - /* clock_gettime() is specified in POSIX.1b (1993). Even so, some systems - * did not support this until much later. CLOCK_MONOTONIC is technically - * optional and may not be supported - but it appears to be universal. - * If this is not supported, provide a system-specific alternate version. */ - struct timespec ts; - clock_gettime(CLOCK_MONOTONIC, &ts); - return ((uint64_t)ts.tv_sec) * 1000000 + ts.tv_nsec / 1000; -} -#endif - -inline uint64_t elapsedUs(monotime start_time) { - return getMonotonicUs() - start_time; -} - -inline uint64_t elapsedMs(monotime start_time) { - return elapsedUs(start_time) / 1000; -} - static int server_math_random(lua_State *L); static int server_math_randomseed(lua_State *L);