Skip to content

Commit

Permalink
Module CLIENT_CHANGE, Fix crash on free blocked client with DB!=0 (re…
Browse files Browse the repository at this point in the history
…dis#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 <binloveplay1314@qq.com>
  • Loading branch information
3 people committed Jul 31, 2023
1 parent d287296 commit 601626e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
7 changes: 0 additions & 7 deletions src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -8094,7 +8094,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;
Expand Down Expand Up @@ -10907,11 +10906,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) {
Expand Down Expand Up @@ -11396,7 +11390,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);
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/moduleapi/hooks.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down

0 comments on commit 601626e

Please sign in to comment.