Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module functions not stopped after trying to block client from MULTI #5567

Open
jeffreylovitz opened this issue Nov 14, 2018 · 1 comment
Open

Comments

@jeffreylovitz
Copy link
Contributor

jeffreylovitz commented Nov 14, 2018

Tested on tip (7721fe8) of unstable and 4.0.11.

If a module attempts to block a client from Lua or MULTI, the internal client handle is set to NULL and an error is sent to the user. This is not visible to the module, however, which continues to function and can modify the keyspace.

127.0.0.1:6379> keys *
(empty list or set)
127.0.0.1:6379> MULTI
OK
127.0.0.1:6379> MODULE_ADD keystring valstring
QUEUED
127.0.0.1:6379> EXEC
1) (error) ERR Blocking module command called from transaction
127.0.0.1:6379> keys *
1) "keystring"
127.0.0.1:6379> get keystring
"valstring"

module.c:

#define REDISMODULE_EXPERIMENTAL_API
#include "redismodule.h"
#include <string.h>

int ModuleCmd(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
  if (argc < 3) return RedisModule_WrongArity(ctx);

  RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx, NULL, NULL, NULL, 0);
  RedisModuleCtx *threadsafectx = RedisModule_GetThreadSafeContext(bc);

  RedisModuleKey *key = RedisModule_OpenKey(threadsafectx, argv[1], REDISMODULE_WRITE);
  RedisModule_StringSet(key, argv[2]);
  RedisModule_CloseKey(key);

  RedisModule_ReplyWithSimpleString(threadsafectx, "Module done!");

  RedisModule_FreeThreadSafeContext(threadsafectx);
  RedisModule_UnblockClient(bc, NULL);

  return REDISMODULE_OK;
}

int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
  if (RedisModule_Init(ctx, "sample", 0, REDISMODULE_APIVER_1) == REDISMODULE_ERR) {
    return REDISMODULE_ERR;
  }
  if(RedisModule_CreateCommand(ctx, "MODULE_ADD", ModuleCmd, "write deny-script", 1, 1, 1) == REDISMODULE_ERR) {
    return REDISMODULE_ERR;
  }

  return REDISMODULE_OK;
}
@oranagra
Copy link
Member

@antirez looks to me that RM_BlockClient should free what it allocated and return NULL in that case, this way the module will be able to know it failed.
i hope it won't cause modules to crash, although today it may cause problems too, if the module continues the execution, unaware of the failure.

@yossigo yossigo mentioned this issue Dec 8, 2020
52 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants