Unverified Commit 5b84dc96 authored by nafraf's avatar nafraf Committed by GitHub
Browse files

Fix module loadex command crash due to invalid config (#13653)

Fix to https://github.com/redis/redis/issues/13650



providing an invalid config to a module with datatype crashes when redis
tries to unload the module due to the invalid config

---------
Co-authored-by: default avatardebing.sun <debing.sun@redis.com>
parent 701f0665
......@@ -12371,7 +12371,7 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa
}
 
if (post_load_err) {
moduleUnload(ctx.module->name, NULL);
serverAssert(moduleUnload(ctx.module->name, NULL, 1) == C_OK);
moduleFreeContext(&ctx);
return C_ERR;
}
......@@ -12387,14 +12387,17 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa
 
/* Unload the module registered with the specified name. On success
* C_OK is returned, otherwise C_ERR is returned and errmsg is set
* with an appropriate message. */
int moduleUnload(sds name, const char **errmsg) {
* with an appropriate message.
* Only forcefully unload this module, passing forced_unload != 0,
* if it is certain that it has not yet been in use (e.g., immediate
* unload on failed load). */
int moduleUnload(sds name, const char **errmsg, int forced_unload) {
struct RedisModule *module = dictFetchValue(modules,name);
 
if (module == NULL) {
*errmsg = "no such module with that name";
return C_ERR;
} else if (listLength(module->types)) {
} else if (listLength(module->types) && !forced_unload) {
*errmsg = "the module exports one or more module-side data "
"types, can't unload";
return C_ERR;
......@@ -13182,7 +13185,7 @@ NULL
 
} else if (!strcasecmp(subcmd,"unload") && c->argc == 3) {
const char *errmsg = NULL;
if (moduleUnload(c->argv[2]->ptr, &errmsg) == C_OK)
if (moduleUnload(c->argv[2]->ptr, &errmsg, 0) == C_OK)
addReply(c,shared.ok);
else {
if (errmsg == NULL) errmsg = "operation not possible.";
......
......@@ -2510,7 +2510,7 @@ void moduleInitModulesSystem(void);
void moduleInitModulesSystemLast(void);
void modulesCron(void);
int moduleLoad(const char *path, void **argv, int argc, int is_loadex);
int moduleUnload(sds name, const char **errmsg);
int moduleUnload(sds name, const char **errmsg, int forced_unload);
void moduleLoadFromQueue(void);
int moduleGetCommandKeysViaAPI(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result);
int moduleGetCommandChannelsViaAPI(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result);
......
......@@ -312,3 +312,12 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_OK;
}
int RedisModule_OnUnload(RedisModuleCtx *ctx) {
REDISMODULE_NOT_USED(ctx);
if (datatype) {
RedisModule_Free(datatype);
datatype = NULL;
}
return REDISMODULE_OK;
}
set testmodule [file normalize tests/modules/datatype.so]
start_server {tags {"modules"}} {
test {DataType: test loadex with invalid config} {
catch { r module loadex $testmodule CONFIG invalid_config 1 } e
assert_match {*ERR Error loading the extension*} $e
}
r module load $testmodule
test {DataType: Test module is sane, GET/SET work.} {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment