From 1907e0f18faa29c28df57d9d21982c5d8a942013 Mon Sep 17 00:00:00 2001 From: Guy Benoish Date: Sun, 29 Mar 2020 17:50:42 +0300 Subject: [PATCH 1/7] PERSIST should notify a keyspace event --- src/expire.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/expire.c b/src/expire.c index 5aff72ee0..c102a01ff 100644 --- a/src/expire.c +++ b/src/expire.c @@ -590,6 +590,7 @@ void pttlCommand(client *c) { void persistCommand(client *c) { if (lookupKeyWrite(c->db,c->argv[1])) { if (removeExpire(c->db,c->argv[1])) { + notifyKeyspaceEvent(NOTIFY_GENERIC,"persist",c->argv[1],c->db->id); addReply(c,shared.cone); server.dirty++; } else { From 4395889c9e13b0f0909372780fd1d6ecad7fb61a Mon Sep 17 00:00:00 2001 From: hwware Date: Sun, 29 Mar 2020 23:06:50 -0400 Subject: [PATCH 2/7] add check for not providing both optin optout flag --- src/networking.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/networking.c b/src/networking.c index 3c754c376..f8808f08f 100644 --- a/src/networking.c +++ b/src/networking.c @@ -2326,6 +2326,14 @@ NULL return; } + if ((options & CLIENT_TRACKING_OPTIN) && (options & CLIENT_TRACKING_OPTOUT)) + { + addReplyError(c, + "You can't specify both OPTIN mode and OPTOUT mode"); + zfree(prefix); + return; + } + enableTracking(c,redir,options,prefix,numprefix); } else if (!strcasecmp(c->argv[2]->ptr,"off")) { disableTracking(c); From b35407fa7c5e43ce184c89abb50134e816292907 Mon Sep 17 00:00:00 2001 From: hwware Date: Sun, 29 Mar 2020 23:20:54 -0400 Subject: [PATCH 3/7] add check for not switching between optin optout mode directly --- src/networking.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/networking.c b/src/networking.c index f8808f08f..85c640e34 100644 --- a/src/networking.c +++ b/src/networking.c @@ -2326,7 +2326,7 @@ NULL return; } - if ((options & CLIENT_TRACKING_OPTIN) && (options & CLIENT_TRACKING_OPTOUT)) + if (options & CLIENT_TRACKING_OPTIN && options & CLIENT_TRACKING_OPTOUT) { addReplyError(c, "You can't specify both OPTIN mode and OPTOUT mode"); @@ -2334,6 +2334,17 @@ NULL return; } + if ((options & CLIENT_TRACKING_OPTIN && c->flags & CLIENT_TRACKING_OPTOUT) || + (options & CLIENT_TRACKING_OPTOUT && c->flags & CLIENT_TRACKING_OPTIN)) + { + addReplyError(c, + "You can't switch OPTIN/OPTOUT mode before disabling " + "tracking for this client, and then re-enabling it with " + "a different mode."); + zfree(prefix); + return; + } + enableTracking(c,redir,options,prefix,numprefix); } else if (!strcasecmp(c->argv[2]->ptr,"off")) { disableTracking(c); From 8cdc153f58e535eff4be5f8f5483794c4cfeec3c Mon Sep 17 00:00:00 2001 From: Valentino Geron Date: Wed, 25 Mar 2020 21:54:14 +0200 Subject: [PATCH 4/7] XACK should be executed in a "all or nothing" fashion. First, we must parse the IDs, so that we abort ASAP. The return value of this command cannot be an error if the client successfully acknowledged some messages, so it should be executed in a "all or nothing" fashion. --- src/t_stream.c | 12 +++++++++++- tests/unit/type/stream-cgroups.tcl | 12 ++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/t_stream.c b/src/t_stream.c index 00d1cbf1c..3f8cbfcfa 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -1922,11 +1922,21 @@ void xackCommand(client *c) { return; } + /* Start parsing the IDs, so that we abort ASAP if there is a syntax + * error: the return value of this command cannot be an error in case + * the client successfully acknowledged some messages, so it should be + * executed in a "all or nothing" fashion. */ + for (int j = 3; j < c->argc; j++) { + streamID id; + if (streamParseStrictIDOrReply(c,c->argv[j],&id,0) != C_OK) return; + } + int acknowledged = 0; for (int j = 3; j < c->argc; j++) { streamID id; unsigned char buf[sizeof(streamID)]; - if (streamParseStrictIDOrReply(c,c->argv[j],&id,0) != C_OK) return; + if (streamParseStrictIDOrReply(c,c->argv[j],&id,0) != C_OK) + serverPanic("StreamID invalid after check. Should not be possible."); streamEncodeID(buf,&id); /* Lookup the ID in the group PEL: it will have a reference to the diff --git a/tests/unit/type/stream-cgroups.tcl b/tests/unit/type/stream-cgroups.tcl index a27e1f582..04661707b 100644 --- a/tests/unit/type/stream-cgroups.tcl +++ b/tests/unit/type/stream-cgroups.tcl @@ -93,6 +93,18 @@ start_server { assert {[r XACK mystream mygroup $id1 $id2] eq 1} } + test {XACK should fail if got at least one invalid ID} { + r del mystream + r xgroup create s g $ MKSTREAM + r xadd s * f1 v1 + set c [llength [lindex [r xreadgroup group g c streams s >] 0 1]] + assert {$c == 1} + set pending [r xpending s g - + 10 c] + set id1 [lindex $pending 0 0] + assert_error "*Invalid stream ID specified*" {r xack s g $id1 invalid-id} + assert {[r xack s g $id1] eq 1} + } + test {PEL NACK reassignment after XGROUP SETID event} { r del events r xadd events * f1 v1 From cd2b5df9712c2a5956d288a762a89eb32f06bd1f Mon Sep 17 00:00:00 2001 From: hwware Date: Wed, 18 Mar 2020 09:48:03 -0400 Subject: [PATCH 5/7] fix spelling in cluster.c --- src/cluster.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cluster.c b/src/cluster.c index a2e9ff5b6..385ff5763 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -933,7 +933,7 @@ int clusterAddNode(clusterNode *node) { return (retval == DICT_OK) ? C_OK : C_ERR; } -/* Remove a node from the cluster. The functio performs the high level +/* Remove a node from the cluster. The function performs the high level * cleanup, calling freeClusterNode() for the low level cleanup. * Here we do the following: * From 15c9e79a7ddc00156ab67ba2afbd2fbf01c981ed Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 7 Mar 2020 10:43:41 +0000 Subject: [PATCH 6/7] debug, dump registers on arm too. --- src/debug.c | 82 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 27 deletions(-) diff --git a/src/debug.c b/src/debug.c index 83e5b6197..c7d5c5336 100644 --- a/src/debug.c +++ b/src/debug.c @@ -1047,6 +1047,61 @@ void logRegisters(ucontext_t *uc) { (unsigned long) uc->uc_mcontext.gregs[18] ); logStackContent((void**)uc->uc_mcontext.gregs[15]); + #elif defined(__aarch64__) /* Linux AArch64 */ + serverLog(LL_WARNING, + "\n" + "X18:%016lx X19:%016lx\nX20:%016lx X21:%016lx\n" + "X22:%016lx X23:%016lx\nX24:%016lx X25:%016lx\n" + "X26:%016lx X27:%016lx\nX28:%016lx X29:%016lx\n" + "X30:%016lx\n" + "pc:%016lx sp:%016lx\npstate:%016lx fault_address:%016lx\n", + (unsigned long) uc->uc_mcontext.regs[18], + (unsigned long) uc->uc_mcontext.regs[19], + (unsigned long) uc->uc_mcontext.regs[20], + (unsigned long) uc->uc_mcontext.regs[21], + (unsigned long) uc->uc_mcontext.regs[22], + (unsigned long) uc->uc_mcontext.regs[23], + (unsigned long) uc->uc_mcontext.regs[24], + (unsigned long) uc->uc_mcontext.regs[25], + (unsigned long) uc->uc_mcontext.regs[26], + (unsigned long) uc->uc_mcontext.regs[27], + (unsigned long) uc->uc_mcontext.regs[28], + (unsigned long) uc->uc_mcontext.regs[29], + (unsigned long) uc->uc_mcontext.regs[30], + (unsigned long) uc->uc_mcontext.pc, + (unsigned long) uc->uc_mcontext.sp, + (unsigned long) uc->uc_mcontext.pstate, + (unsigned long) uc->uc_mcontext.fault_address + ); + logStackContent((void**)uc->uc_mcontext.sp); + #elif defined(__arm__) /* Linux ARM */ + serverLog(LL_WARNING, + "\n" + "R10:%016lx R9 :%016lx\nR8 :%016lx R7 :%016lx\n" + "R6 :%016lx R5 :%016lx\nR4 :%016lx R3 :%016lx\n" + "R2 :%016lx R1 :%016lx\nR0 :%016lx EC :%016lx\n" + "fp: %016lx ip:%016lx\n", + "pc:%016lx sp:%016lx\ncpsr:%016lx fault_address:%016lx\n", + (unsigned long) uc->uc_mcontext.arm_r10, + (unsigned long) uc->uc_mcontext.arm_r9, + (unsigned long) uc->uc_mcontext.arm_r8, + (unsigned long) uc->uc_mcontext.arm_r7, + (unsigned long) uc->uc_mcontext.arm_r6, + (unsigned long) uc->uc_mcontext.arm_r5, + (unsigned long) uc->uc_mcontext.arm_r4, + (unsigned long) uc->uc_mcontext.arm_r3, + (unsigned long) uc->uc_mcontext.arm_r2, + (unsigned long) uc->uc_mcontext.arm_r1, + (unsigned long) uc->uc_mcontext.arm_r0, + (unsigned long) uc->uc_mcontext.error_code, + (unsigned long) uc->uc_mcontext.arm_fp, + (unsigned long) uc->uc_mcontext.arm_ip, + (unsigned long) uc->uc_mcontext.arm_pc, + (unsigned long) uc->uc_mcontext.arm_sp, + (unsigned long) uc->uc_mcontext.arm_cpsr, + (unsigned long) uc->uc_mcontext.fault_address + ); + logStackContent((void**)uc->uc_mcontext.arm_sp); #endif #elif defined(__FreeBSD__) #if defined(__x86_64__) @@ -1187,33 +1242,6 @@ void logRegisters(ucontext_t *uc) { (unsigned long) uc->uc_mcontext.mc_cs ); logStackContent((void**)uc->uc_mcontext.mc_rsp); -#elif defined(__aarch64__) /* Linux AArch64 */ - serverLog(LL_WARNING, - "\n" - "X18:%016lx X19:%016lx\nX20:%016lx X21:%016lx\n" - "X22:%016lx X23:%016lx\nX24:%016lx X25:%016lx\n" - "X26:%016lx X27:%016lx\nX28:%016lx X29:%016lx\n" - "X30:%016lx\n" - "pc:%016lx sp:%016lx\npstate:%016lx fault_address:%016lx\n", - (unsigned long) uc->uc_mcontext.regs[18], - (unsigned long) uc->uc_mcontext.regs[19], - (unsigned long) uc->uc_mcontext.regs[20], - (unsigned long) uc->uc_mcontext.regs[21], - (unsigned long) uc->uc_mcontext.regs[22], - (unsigned long) uc->uc_mcontext.regs[23], - (unsigned long) uc->uc_mcontext.regs[24], - (unsigned long) uc->uc_mcontext.regs[25], - (unsigned long) uc->uc_mcontext.regs[26], - (unsigned long) uc->uc_mcontext.regs[27], - (unsigned long) uc->uc_mcontext.regs[28], - (unsigned long) uc->uc_mcontext.regs[29], - (unsigned long) uc->uc_mcontext.regs[30], - (unsigned long) uc->uc_mcontext.pc, - (unsigned long) uc->uc_mcontext.sp, - (unsigned long) uc->uc_mcontext.pstate, - (unsigned long) uc->uc_mcontext.fault_address - ); - logStackContent((void**)uc->uc_mcontext.sp); #else serverLog(LL_WARNING, " Dumping of registers not supported for this OS/arch"); From eba28e2cea0b2632cf751426ada02adf24f273db Mon Sep 17 00:00:00 2001 From: Guy Benoish Date: Thu, 30 Jan 2020 19:15:12 +0530 Subject: [PATCH 7/7] DEBUG OBJECT should pass keyname to module when loading --- src/debug.c | 2 +- src/rdb.c | 4 ++-- src/rdb.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/debug.c b/src/debug.c index c7d5c5336..baaaa2424 100644 --- a/src/debug.c +++ b/src/debug.c @@ -490,7 +490,7 @@ NULL "encoding:%s serializedlength:%zu " "lru:%d lru_seconds_idle:%llu%s", (void*)val, val->refcount, - strenc, rdbSavedObjectLen(val), + strenc, rdbSavedObjectLen(val, c->argv[2]), val->lru, estimateObjectIdleTime(val)/1000, extra); } else if (!strcasecmp(c->argv[1]->ptr,"sdslen") && c->argc == 3) { dictEntry *de; diff --git a/src/rdb.c b/src/rdb.c index 5d34f5a32..fc8911979 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1002,8 +1002,8 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key) { * the rdbSaveObject() function. Currently we use a trick to get * this length with very little changes to the code. In the future * we could switch to a faster solution. */ -size_t rdbSavedObjectLen(robj *o) { - ssize_t len = rdbSaveObject(NULL,o,NULL); +size_t rdbSavedObjectLen(robj *o, robj *key) { + ssize_t len = rdbSaveObject(NULL,o,key); serverAssertWithInfo(NULL,o,len != -1); return len; } diff --git a/src/rdb.h b/src/rdb.h index 4229beea8..b276a978b 100644 --- a/src/rdb.h +++ b/src/rdb.h @@ -143,7 +143,7 @@ int rdbSaveToSlavesSockets(rdbSaveInfo *rsi); void rdbRemoveTempFile(pid_t childpid); int rdbSave(char *filename, rdbSaveInfo *rsi); ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key); -size_t rdbSavedObjectLen(robj *o); +size_t rdbSavedObjectLen(robj *o, robj *key); robj *rdbLoadObject(int type, rio *rdb, robj *key); void backgroundSaveDoneHandler(int exitcode, int bysignal); int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, long long expiretime);