upstream/redis
1
0
Fork 0
mirror of https://github.com/redis/redis.git synced 2026-05-28 04:02:46 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Fix sharded pubsub unsubscribe lookup using cached command slot (#15094)

Browse source 
Fixes #15085 

## Problem
getKeySlot() may return `server.current_client->slot` while a command is
executing instead of computing the slot from the provided string.

The unsubscribe can be triggered by another client, in which case server.current_client is not the client being unsubscribed, so getKeySlot() would return that client's cached slot. Using this wrong slot would make the lookup in type.serverPubSubChannels miss the channel and ultimately trigger the assertion below.

## Fix
Always use keyHashSlot() instead of getKeySlot() on unsubscribe.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
This commit is contained in:
Shubham S Taple 2026-04-29 19:34:06 +05:30 committed by GitHub
parent 48eaa75257
commit 0bbb196c46
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 31 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/pubsub.c
View file

@ -293,7 +293,10 @@ int pubsubUnsubscribeChannel(client *c, robj *channel, int notify, pubsubtype ty
retval = 1;
/* Remove the client from the channel -> clients list hash table */
if (server.cluster_enabled && type.shard) {
slot = getKeySlot(channel->ptr);
/* Compute the slot from the channel directly instead of using getKeySlot(),
* because the unsubscribe may be triggered by a different client, and
* getKeySlot() would return the cached slot of that client. */
slot = keyHashSlot(channel->ptr, sdslen(channel->ptr));
}
de = kvstoreDictFind(*type.serverPubSubChannels, slot, channel);
serverAssertWithInfo(c,NULL,de != NULL);

27
tests/unit/cluster/sharded-pubsub.tcl
View file

@ -64,4 +64,31 @@ start_cluster 1 1 {tags {external:skip cluster}} {
catch {[$replica EXEC]} err
assert_match {EXECABORT*} $err
}
# Regression: shard channel slot must not follow getKeySlot() current_client
# cache when CLIENT KILL runs inside another client's EXEC (pubsubUnsubscribeChannel).
test {Shard pubsub: CLIENT KILL subscriber inside MULTI/EXEC (cross-slot)} {
# SET fixes the transaction client's slot to keyk's slot; the subscriber must
# use a shard channel in a different slot so a wrong-slot lookup would fail.
set keyk "{06S}k"
set channel "{Qi}ch"
assert {[R 0 cluster keyslot $channel] != [R 0 cluster keyslot $keyk]}
set rd_sub [redis_deferring_client]
$rd_sub client id
set cid [$rd_sub read]
$rd_sub ssubscribe $channel
$rd_sub read
$primary multi
$primary set $keyk v
$primary client kill id $cid
set got [$primary exec]
assert_equal {OK 1} $got
assert_equal PONG [$primary ping]
catch {$rd_sub read}
$rd_sub close
}
}