Unverified Commit 24fdc87f authored by Salvatore Sanfilippo's avatar Salvatore Sanfilippo Committed by GitHub
Browse files

Merge pull request #4994 from soloestoy/module-blocked-client

Modules: make unloading module more safe
parents 6e98214f c74398e1
...@@ -63,6 +63,7 @@ struct RedisModule { ...@@ -63,6 +63,7 @@ struct RedisModule {
int in_call; /* RM_Call() nesting level */ int in_call; /* RM_Call() nesting level */
int in_hook; /* Hooks callback nesting level for this module (0 or 1). */ int in_hook; /* Hooks callback nesting level for this module (0 or 1). */
int options; /* Module options and capabilities. */ int options; /* Module options and capabilities. */
int blocked_clients; /* Count of RedisModuleBlockedClient in this module. */
RedisModuleInfoFunc info_cb; /* Callback for module to add INFO fields. */ RedisModuleInfoFunc info_cb; /* Callback for module to add INFO fields. */
}; };
typedef struct RedisModule RedisModule; typedef struct RedisModule RedisModule;
...@@ -3961,6 +3962,7 @@ RedisModuleBlockedClient *RM_BlockClient(RedisModuleCtx *ctx, RedisModuleCmdFunc ...@@ -3961,6 +3962,7 @@ RedisModuleBlockedClient *RM_BlockClient(RedisModuleCtx *ctx, RedisModuleCmdFunc
c->bpop.module_blocked_handle = zmalloc(sizeof(RedisModuleBlockedClient)); c->bpop.module_blocked_handle = zmalloc(sizeof(RedisModuleBlockedClient));
RedisModuleBlockedClient *bc = c->bpop.module_blocked_handle; RedisModuleBlockedClient *bc = c->bpop.module_blocked_handle;
ctx->module->blocked_clients++;
/* We need to handle the invalid operation of calling modules blocking /* We need to handle the invalid operation of calling modules blocking
* commands from Lua or MULTI. We actually create an already aborted * commands from Lua or MULTI. We actually create an already aborted
...@@ -4119,6 +4121,7 @@ void moduleHandleBlockedClients(void) { ...@@ -4119,6 +4121,7 @@ void moduleHandleBlockedClients(void) {
/* Free 'bc' only after unblocking the client, since it is /* Free 'bc' only after unblocking the client, since it is
* referenced in the client blocking context, and must be valid * referenced in the client blocking context, and must be valid
* when calling unblockClient(). */ * when calling unblockClient(). */
bc->module->blocked_clients--;
zfree(bc); zfree(bc);
/* Lock again before to iterate the loop. */ /* Lock again before to iterate the loop. */
...@@ -6089,6 +6092,7 @@ int moduleLoad(const char *path, void **module_argv, int module_argc) { ...@@ -6089,6 +6092,7 @@ int moduleLoad(const char *path, void **module_argv, int module_argc) {
/* Redis module loaded! Register it. */ /* Redis module loaded! Register it. */
dictAdd(modules,ctx.module->name,ctx.module); dictAdd(modules,ctx.module->name,ctx.module);
ctx.module->blocked_clients = 0;
ctx.module->handle = handle; ctx.module->handle = handle;
serverLog(LL_NOTICE,"Module '%s' loaded from %s",ctx.module->name,path); serverLog(LL_NOTICE,"Module '%s' loaded from %s",ctx.module->name,path);
moduleFreeContext(&ctx); moduleFreeContext(&ctx);
...@@ -6114,6 +6118,9 @@ int moduleUnload(sds name) { ...@@ -6114,6 +6118,9 @@ int moduleUnload(sds name) {
} else if (listLength(module->usedby)) { } else if (listLength(module->usedby)) {
errno = EPERM; errno = EPERM;
return REDISMODULE_ERR; return REDISMODULE_ERR;
} else if (module->blocked_clients) {
errno = EAGAIN;
return REDISMODULE_ERR;
} }
/* Give module a chance to clean up. */ /* Give module a chance to clean up. */
...@@ -6279,6 +6286,10 @@ NULL ...@@ -6279,6 +6286,10 @@ NULL
errmsg = "the module exports APIs used by other modules. " errmsg = "the module exports APIs used by other modules. "
"Please unload them first and try again"; "Please unload them first and try again";
break; break;
case EAGAIN:
errmsg = "the module has blocked clients. "
"Please wait them unblocked and try again";
break;
default: default:
errmsg = "operation not possible."; errmsg = "operation not possible.";
break; break;
......
...@@ -2104,7 +2104,7 @@ void beforeSleep(struct aeEventLoop *eventLoop) { ...@@ -2104,7 +2104,7 @@ void beforeSleep(struct aeEventLoop *eventLoop) {
/* Check if there are clients unblocked by modules that implement /* Check if there are clients unblocked by modules that implement
* blocking commands. */ * blocking commands. */
moduleHandleBlockedClients(); if (moduleCount()) moduleHandleBlockedClients();
/* Try to process pending commands for clients that were just unblocked. */ /* Try to process pending commands for clients that were just unblocked. */
if (listLength(server.unblocked_clients)) if (listLength(server.unblocked_clients))
......
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