From 7ad786db2ee61e33b6a96de8c50b53785f56f556 Mon Sep 17 00:00:00 2001 From: uriyage <78144248+uriyage@users.noreply.github.com> Date: Tue, 15 Nov 2022 00:40:35 +0200 Subject: [PATCH] Module CLIENT_CHANGE, Fix crash on free blocked client with DB!=0 (#11500) In moduleFireServerEvent we change the real client DB to 0 on freeClient in case the event is REDISMODULE_EVENT_CLIENT_CHANGE. It results in a crash if the client is blocked on a key on other than DB 0. The DB change is not necessary even for module-client, as we set its DB to 0 on either createClient or moduleReleaseTempClient. Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com> Co-authored-by: Binbin (cherry picked from commit e4eb18b303d716aeebc4153176a6cd93b8bd5d66) --- src/module.c | 7 ------- tests/unit/moduleapi/hooks.tcl | 13 +++++++++++++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/module.c b/src/module.c index e2a9bd037..e4e54d598 100644 --- a/src/module.c +++ b/src/module.c @@ -7974,7 +7974,6 @@ void moduleCallClusterReceivers(const char *sender_id, uint64_t module_id, uint8 if (r->module_id == module_id) { RedisModuleCtx ctx; moduleCreateContext(&ctx, r->module, REDISMODULE_CTX_TEMP_CLIENT); - selectDb(ctx.client, 0); r->callback(&ctx,sender_id,type,payload,len); moduleFreeContext(&ctx); return; @@ -10787,11 +10786,6 @@ void moduleFireServerEvent(uint64_t eid, int subid, void *data) { RedisModuleClientInfoV1 civ1; RedisModuleReplicationInfoV1 riv1; RedisModuleModuleChangeV1 mcv1; - /* Start at DB zero by default when calling the handler. It's - * up to the specific event setup to change it when it makes - * sense. For instance for FLUSHDB events we select the correct - * DB automatically. */ - selectDb(ctx.client, 0); /* Event specific context and data pointer setup. */ if (eid == REDISMODULE_EVENT_CLIENT_CHANGE) { @@ -11275,7 +11269,6 @@ int moduleLoad(const char *path, void **module_argv, int module_argc, int is_loa } RedisModuleCtx ctx; moduleCreateContext(&ctx, NULL, REDISMODULE_CTX_TEMP_CLIENT); /* We pass NULL since we don't have a module yet. */ - selectDb(ctx.client, 0); if (onload((void*)&ctx,module_argv,module_argc) == REDISMODULE_ERR) { serverLog(LL_WARNING, "Module %s initialization failed. Module not loaded",path); diff --git a/tests/unit/moduleapi/hooks.tcl b/tests/unit/moduleapi/hooks.tcl index 814f31bc0..6e79f942e 100644 --- a/tests/unit/moduleapi/hooks.tcl +++ b/tests/unit/moduleapi/hooks.tcl @@ -15,6 +15,19 @@ tags "modules" { assert {[r hooks.event_count client-disconnected] > 1} } + test {Test module client change event for blocked client} { + set rd [redis_deferring_client] + # select db other than 0 + $rd select 1 + # block on key + $rd brpop foo 0 + # kill blocked client + r client kill skipme yes + # assert server is still up + assert_equal [r ping] PONG + $rd close + } + test {Test module cron hook} { after 100 assert {[r hooks.event_count cron-loop] > 0}