From 6e5efbf9181eeff64eaa5e549646d914f64d2242 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 02cfb2cb1d666f5fed171b2755c5317cc726d454 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 74778c8a0174e993b3f630e426eb4202648dc9e1 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 1f6160ccb24b58c4dd2a0e14c84ee98c91d11cea 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 71f6e21cea5458c762bd6327902fd86dc2eb81d7 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 e763a8debf1e6be6c795d9d3bbe37cc089e6690e 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 71134e357ffd6ced7c40c145205dbbac173ee181 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);