From cf860df59921efcc74be410bdf165abd784df502 Mon Sep 17 00:00:00 2001 From: Shaya Potter Date: Thu, 21 Oct 2021 14:01:10 +0300 Subject: [PATCH] Fix module blocked clients RESP version (#9634) Before this commit, module blocked clients did not carry through the original RESP version, resulting with RESP3 clients receiving unexpected RESP2 replies. --- src/module.c | 7 ++++++- tests/modules/blockedclient.c | 18 ++++++++++++++++++ tests/unit/moduleapi/blockedclient.tcl | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/module.c b/src/module.c index 5fa0162d5..ff7e1c0b8 100644 --- a/src/module.c +++ b/src/module.c @@ -6181,6 +6181,8 @@ RedisModuleBlockedClient *moduleBlockClient(RedisModuleCtx *ctx, RedisModuleCmdF bc->free_privdata = free_privdata; bc->privdata = privdata; bc->reply_client = createClient(NULL); + if (bc->client) + bc->reply_client->resp = bc->client->resp; bc->reply_client->flags |= CLIENT_MODULE; bc->dbid = c->db->id; bc->blocked_on_keys = keys != NULL; @@ -6654,7 +6656,10 @@ RedisModuleCtx *RM_GetThreadSafeContext(RedisModuleBlockedClient *bc) { ctx->client = createClient(NULL); if (bc) { selectDb(ctx->client,bc->dbid); - if (bc->client) ctx->client->id = bc->client->id; + if (bc->client) { + ctx->client->id = bc->client->id; + ctx->client->resp = bc->client->resp; + } } return ctx; } diff --git a/tests/modules/blockedclient.c b/tests/modules/blockedclient.c index c3b0d5eb4..ebe89ecb2 100644 --- a/tests/modules/blockedclient.c +++ b/tests/modules/blockedclient.c @@ -188,6 +188,21 @@ int do_rm_call(RedisModuleCtx *ctx, RedisModuleString **argv, int argc){ return REDISMODULE_OK; } +/* simulate a blocked client replying to a thread safe context without creating a thread */ +int do_fake_bg_true(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + UNUSED(argv); + UNUSED(argc); + + RedisModuleBlockedClient *bc = RedisModule_BlockClient(ctx, NULL, NULL, NULL, 0); + RedisModuleCtx *bctx = RedisModule_GetThreadSafeContext(bc); + + RedisModule_ReplyWithBool(bctx, 1); + + RedisModule_FreeThreadSafeContext(bctx); + RedisModule_UnblockClient(bc, NULL); + + return REDISMODULE_OK; +} int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { REDISMODULE_NOT_USED(argv); @@ -205,5 +220,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) if (RedisModule_CreateCommand(ctx, "do_bg_rm_call", do_bg_rm_call, "", 0, 0, 0) == REDISMODULE_ERR) return REDISMODULE_ERR; + if (RedisModule_CreateCommand(ctx, "do_fake_bg_true", do_fake_bg_true, "", 0, 0, 0) == REDISMODULE_ERR) + return REDISMODULE_ERR; + return REDISMODULE_OK; } diff --git a/tests/unit/moduleapi/blockedclient.tcl b/tests/unit/moduleapi/blockedclient.tcl index 0598a84d6..8f657afc5 100644 --- a/tests/unit/moduleapi/blockedclient.tcl +++ b/tests/unit/moduleapi/blockedclient.tcl @@ -78,6 +78,20 @@ start_server {tags {"modules"}} { r do_bg_rm_call hgetall hash } {foo bar} + test {RESP version carries through to blocked client} { + for {set client_proto 2} {$client_proto <= 3} {incr client_proto} { + r hello $client_proto + r readraw 1 + set ret [r do_fake_bg_true] + if {$client_proto == 2} { + assert_equal $ret {:1} + } else { + assert_equal $ret "#t" + } + r readraw 0 + } + } + test {blocked client reaches client output buffer limit} { r hset hash big [string repeat x 50000] r hset hash bada [string repeat x 50000]