From 665da8a0569628bcb581e40f8014334f6fbd575b Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 23 Feb 2021 12:57:45 +0200 Subject: [PATCH 01/24] Fix failed tests on Linux Alpine and add a CI job. (#8532) * Remove linux/version.h dependency. This introduces unnecessary dependencies, and generally not a good idea as the platform we build on may be different than the platform we run on. To determine if sync_file_range exists we can simply rely on header file hints. * Fix setproctitle() on libmusl. The previous ifdef checks were a bit too strict for no apparent reason. * Fix tests failure on Linux with no backtrace. * Add alpine daily CI job. --- .github/workflows/daily.yml | 20 ++++++++++++++++++++ src/config.h | 17 ++--------------- src/fmacros.h | 1 - src/setproctitle.c | 2 +- tests/integration/logging.tcl | 14 +++++++++++++- 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 9d7eb65c9..6aa535c5e 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -221,3 +221,23 @@ jobs: MAKE=gmake ./runtest-moduleapi --verbose && ./runtest-sentinel && ./runtest-cluster + + test-alpine: + runs-on: ubuntu-latest + container: alpine:latest + steps: + - uses: actions/checkout@v2 + - name: make + run: | + apk add build-base + make REDIS_CFLAGS='-Werror' + - name: test + run: | + apk add tcl procps + ./runtest --accurate --verbose --dump-logs + - name: module api test + run: ./runtest-moduleapi --verbose + - name: sentinel tests + run: ./runtest-sentinel + - name: cluster tests + run: ./runtest-cluster diff --git a/src/config.h b/src/config.h index 1d4cb5835..56c1ab6ae 100644 --- a/src/config.h +++ b/src/config.h @@ -35,7 +35,6 @@ #endif #ifdef __linux__ -#include #include #endif @@ -114,19 +113,7 @@ /* Define rdb_fsync_range to sync_file_range() on Linux, otherwise we use * the plain fsync() call. */ -#ifdef __linux__ -#if defined(__GLIBC__) && defined(__GLIBC_PREREQ) -#if (LINUX_VERSION_CODE >= 0x020611 && __GLIBC_PREREQ(2, 6)) -#define HAVE_SYNC_FILE_RANGE 1 -#endif -#else -#if (LINUX_VERSION_CODE >= 0x020611) -#define HAVE_SYNC_FILE_RANGE 1 -#endif -#endif -#endif - -#ifdef HAVE_SYNC_FILE_RANGE +#if (defined(__linux__) && defined(SYNC_FILE_RANGE_WAIT_BEFORE)) #define rdb_fsync_range(fd,off,size) sync_file_range(fd,off,size,SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE) #else #define rdb_fsync_range(fd,off,size) fsync(fd) @@ -143,7 +130,7 @@ #define ESOCKTNOSUPPORT 0 #endif -#if ((defined __linux && defined(__GLIBC__)) || defined __APPLE__) +#if (defined __linux || defined __APPLE__) #define USE_SETPROCTITLE #define INIT_SETPROCTITLE_REPLACEMENT void spt_init(int argc, char *argv[]); diff --git a/src/fmacros.h b/src/fmacros.h index 089dc8de7..a97d21a47 100644 --- a/src/fmacros.h +++ b/src/fmacros.h @@ -60,7 +60,6 @@ #ifdef __linux__ /* features.h uses the defines above to set feature specific defines. */ -#include #include #endif diff --git a/src/setproctitle.c b/src/setproctitle.c index 1c91570eb..019402348 100644 --- a/src/setproctitle.c +++ b/src/setproctitle.c @@ -232,7 +232,7 @@ void spt_init(int argc, char *argv[]) { if (!(SPT.arg0 = strdup(argv[0]))) goto syerr; -#if __GLIBC__ +#if __linux__ if (!(tmp = strdup(program_invocation_name))) goto syerr; diff --git a/tests/integration/logging.tcl b/tests/integration/logging.tcl index fd9034644..fec14ff2b 100644 --- a/tests/integration/logging.tcl +++ b/tests/integration/logging.tcl @@ -1,6 +1,18 @@ set system_name [string tolower [exec uname -s]] +set system_supported 0 -if {$system_name eq {linux} || $system_name eq {darwin}} { +# We only support darwin or Linux with glibc +if {$system_name eq {darwin}} { + set system_supported 1 +} elseif {$system_name eq {linux}} { + # Avoid the test on libmusl, which does not support backtrace + set ldd [exec ldd src/redis-server] + if {![string match {*libc.musl*} $ldd]} { + set system_supported 1 + } +} + +if {$system_supported} { set server_path [tmpdir server.log] start_server [list overrides [list dir $server_path]] { test "Server is able to generate a stack trace on selected systems" { From 1cb0f8517249083ef3829be042de8faa056fcbd5 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 23 Feb 2021 17:08:49 +0200 Subject: [PATCH 02/24] Fix compile errors with no HAVE_MALLOC_SIZE. (#8533) Also adds a new daily CI test, relying on the fact that we don't use malloc_size() on alpine libmusl. Fixes #8531 --- .github/workflows/daily.yml | 22 +++++++++++++++++++++- src/zmalloc.c | 7 ++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 6aa535c5e..9b9d15988 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -222,7 +222,7 @@ jobs: ./runtest-sentinel && ./runtest-cluster - test-alpine: + test-alpine-jemalloc: runs-on: ubuntu-latest container: alpine:latest steps: @@ -241,3 +241,23 @@ jobs: run: ./runtest-sentinel - name: cluster tests run: ./runtest-cluster + + test-alpine-libc-malloc: + runs-on: ubuntu-latest + container: alpine:latest + steps: + - uses: actions/checkout@v2 + - name: make + run: | + apk add build-base + make REDIS_CFLAGS='-Werror' USE_JEMALLOC=no + - name: test + run: | + apk add tcl procps + ./runtest --accurate --verbose --dump-logs + - name: module api test + run: ./runtest-moduleapi --verbose + - name: sentinel tests + run: ./runtest-sentinel + - name: cluster tests + run: ./runtest-cluster diff --git a/src/zmalloc.c b/src/zmalloc.c index c8d6c825f..fbac09616 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -32,6 +32,7 @@ #include #include #include +#include /* This function provide us access to the original libc free(). This is useful * for instance to free results obtained by backtrace_symbols(). We need @@ -49,18 +50,14 @@ void zlibc_free(void *ptr) { #ifdef HAVE_MALLOC_SIZE #define PREFIX_SIZE (0) +#define ASSERT_NO_SIZE_OVERFLOW(sz) #else #if defined(__sun) || defined(__sparc) || defined(__sparc__) #define PREFIX_SIZE (sizeof(long long)) #else #define PREFIX_SIZE (sizeof(size_t)) #endif -#endif - -#if PREFIX_SIZE > 0 #define ASSERT_NO_SIZE_OVERFLOW(sz) assert((sz) + PREFIX_SIZE > (sz)) -#else -#define ASSERT_NO_SIZE_OVERFLOW(sz) #endif /* Explicitly override malloc/free etc when using tcmalloc. */ From e209b6c1c9cffa4b6cd5a1ba1394adaf204d6aa9 Mon Sep 17 00:00:00 2001 From: guybe7 Date: Tue, 23 Feb 2021 19:28:03 +0100 Subject: [PATCH 03/24] Fix race in CONFIG REWRITE sanity (#8536) server may still be LOADING the RDB when receiving the ping --- tests/unit/introspection.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index ba28341ff..c1cf72f0c 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -173,7 +173,7 @@ start_server {tags {"introspection"}} { # Rewrite entire configuration, restart and confirm the # server is able to parse it and start. assert_equal [r debug config-rewrite-force-all] "OK" - restart_server 0 false false + restart_server 0 true false assert_equal [r ping] "PONG" # Verify no changes were introduced From 9bf31f67f0182c3524ff7e292bd895c7c26c1e71 Mon Sep 17 00:00:00 2001 From: Huang Zw Date: Wed, 24 Feb 2021 08:55:10 +0800 Subject: [PATCH 04/24] In luaRedisGenericCommand check channel return is ACL_DENIED_CHANNEL (#8535) not ACL_DENIED_AUTH --- src/scripting.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scripting.c b/src/scripting.c index e23d49e8b..7bf9f1c04 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -619,7 +619,7 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) { "at least one of the keys mentioned in the " "command arguments"); break; - case ACL_DENIED_AUTH: + case ACL_DENIED_CHANNEL: luaPushError(lua, "The user executing the script can't publish " "to the channel mentioned in the command"); break; From 0e5e9318391b460e7608730122552f2df4b7d8ab Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Wed, 24 Feb 2021 09:48:04 +0200 Subject: [PATCH 05/24] Use malloc_usable_size() on FreeBSD. (#8545) --- src/zmalloc.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/zmalloc.h b/src/zmalloc.h index 64bc9fc76..6f3f6354a 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -61,9 +61,12 @@ #define zmalloc_size(p) malloc_size(p) #endif +/* On native libc implementations, we should still do our best to provide a + * HAVE_MALLOC_SIZE capability. + */ #ifndef ZMALLOC_LIB #define ZMALLOC_LIB "libc" -#ifdef __GLIBC__ +#if defined(__GLIBC__) || defined(__FreeBSD__) #include #define HAVE_MALLOC_SIZE 1 #define zmalloc_size(p) malloc_usable_size(p) From ba7a99485ab37381baeee233739702deaf74f9d4 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Wed, 24 Feb 2021 10:10:02 +0200 Subject: [PATCH 06/24] Cleanup clang warnings. (#8546) --- src/Makefile | 5 +++++ src/lolwut5.c | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Makefile b/src/Makefile index 3bc9f11c0..ecd69295f 100644 --- a/src/Makefile +++ b/src/Makefile @@ -21,7 +21,12 @@ NODEPS:=clean distclean # Default settings STD=-pedantic -DREDIS_STATIC='' + +# Use -Wno-c11-extensions on clang, either where explicitly used or on +# platforms we can assume it's being used. ifneq (,$(findstring clang,$(CC))) + STD+=-Wno-c11-extensions +else ifneq (,$(findstring FreeBSD,$(uname_S))) STD+=-Wno-c11-extensions endif diff --git a/src/lolwut5.c b/src/lolwut5.c index d64e0bb27..d864888ba 100644 --- a/src/lolwut5.c +++ b/src/lolwut5.c @@ -84,9 +84,9 @@ lwCanvas *lwDrawSchotter(int console_cols, int squares_per_row, int squares_per_ * rows. */ float angle = 0; if (y > 1) { - float r1 = (float)rand() / RAND_MAX / squares_per_col * y; - float r2 = (float)rand() / RAND_MAX / squares_per_col * y; - float r3 = (float)rand() / RAND_MAX / squares_per_col * y; + float r1 = (float)rand() / (float) RAND_MAX / squares_per_col * y; + float r2 = (float)rand() / (float) RAND_MAX / squares_per_col * y; + float r3 = (float)rand() / (float) RAND_MAX / squares_per_col * y; if (rand() % 2) r1 = -r1; if (rand() % 2) r2 = -r2; if (rand() % 2) r3 = -r3; From 7230957b5fbc3b77ec43d86081d23531ce21cf59 Mon Sep 17 00:00:00 2001 From: Huang Zw Date: Wed, 24 Feb 2021 19:18:54 +0800 Subject: [PATCH 07/24] Fix quicklistDelRange dead code, delete range wrong (#8257) This commit fixes a bug in what's currently dead code in redis. In quicklistDelRange when delete entry from entry.offset to node tail, extent only need gte node->count - entry.offset, not node->count Co-authored-by: Yoav Steinberg --- src/quicklist.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/quicklist.c b/src/quicklist.c index c8517414c..7b7aa7839 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -999,7 +999,7 @@ int quicklistDelRange(quicklist *quicklist, const long start, * can just delete the entire node without ziplist math. */ delete_entire_node = 1; del = node->count; - } else if (entry.offset >= 0 && extent >= node->count) { + } else if (entry.offset >= 0 && extent + entry.offset >= node->count) { /* If deleting more nodes after this one, calculate delete based * on size of current node. */ del = node->count - entry.offset; @@ -2238,6 +2238,17 @@ int quicklistTest(int argc, char *argv[]) { quicklistRelease(ql); } + TEST("delete less than fill but across nodes") { + quicklist *ql = quicklistNew(-2, options[_i]); + quicklistSetFill(ql, 32); + for (int i = 0; i < 500; i++) + quicklistPushTail(ql, genstr("hello", i + 1), 32); + ql_verify(ql, 16, 500, 32, 20); + quicklistDelRange(ql, 60, 10); + ql_verify(ql, 16, 490, 32, 20); + quicklistRelease(ql); + } + TEST("delete negative 1 from 500 list") { quicklist *ql = quicklistNew(-2, options[_i]); quicklistSetFill(ql, 32); From c307ce826a9f7068681790afb926647ab448ffba Mon Sep 17 00:00:00 2001 From: guybe7 Date: Wed, 24 Feb 2021 15:41:00 +0100 Subject: [PATCH 08/24] XTRIM should take at least 4 args (#8549) Fix command arity. Other than key name it must at least take the trimming strategy and it's limit --- src/server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server.c b/src/server.c index b22af980b..f4dff6b0e 100644 --- a/src/server.c +++ b/src/server.c @@ -1070,7 +1070,7 @@ struct redisCommand redisCommandTable[] = { "write fast @stream", 0,NULL,1,1,1,0,0,0}, - {"xtrim",xtrimCommand,-2, + {"xtrim",xtrimCommand,-4, "write random @stream", 0,NULL,1,1,1,0,0,0}, From a4e3ddd33cf9f847ee457391a4771edf0644fd2b Mon Sep 17 00:00:00 2001 From: guybe7 Date: Wed, 24 Feb 2021 15:41:50 +0100 Subject: [PATCH 09/24] XTRIM: Parse args before lookupKey (#8550) This aligns better with other commands, specifically XADD --- src/t_stream.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/t_stream.c b/src/t_stream.c index d9db97c3c..4a425fd78 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -3229,17 +3229,17 @@ cleanup: void xtrimCommand(client *c) { robj *o; + /* Argument parsing. */ + streamAddTrimArgs parsed_args; + if (streamParseAddOrTrimArgsOrReply(c, &parsed_args, 1) < 0) + return; /* streamParseAddOrTrimArgsOrReply already replied. */ + /* If the key does not exist, we are ok returning zero, that is, the * number of elements removed from the stream. */ if ((o = lookupKeyWriteOrReply(c,c->argv[1],shared.czero)) == NULL || checkType(c,o,OBJ_STREAM)) return; stream *s = o->ptr; - /* Argument parsing. */ - streamAddTrimArgs parsed_args; - if (streamParseAddOrTrimArgsOrReply(c, &parsed_args, 1) < 0) - return; /* streamParseAddOrTrimArgsOrReply already replied. */ - /* Perform the trimming. */ int64_t deleted = streamTrim(s, &parsed_args); if (deleted) { From c65e914491cd5e676931bcd75d4a6f9052e43abe Mon Sep 17 00:00:00 2001 From: sundb Date: Thu, 25 Feb 2021 00:45:13 +0800 Subject: [PATCH 10/24] Use addReplyErrorObject with shared.noscripterr (#8544) --- src/scripting.c | 4 ++-- tests/unit/info.tcl | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/scripting.c b/src/scripting.c index 7bf9f1c04..6830e7a70 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -1544,7 +1544,7 @@ void evalGenericCommand(client *c, int evalsha) { * return an error. */ if (evalsha) { lua_pop(lua,1); /* remove the error handler from the stack. */ - addReply(c, shared.noscripterr); + addReplyErrorObject(c, shared.noscripterr); return; } if (luaCreateFunction(c,lua,c->argv[1]) == NULL) { @@ -1695,7 +1695,7 @@ void evalShaCommand(client *c) { * not the right length. So we return an error ASAP, this way * evalGenericCommand() can be implemented without string length * sanity check */ - addReply(c, shared.noscripterr); + addReplyErrorObject(c, shared.noscripterr); return; } if (!(c->flags & CLIENT_LUA_DEBUG)) diff --git a/tests/unit/info.tcl b/tests/unit/info.tcl index 08171fff9..0602e7147 100644 --- a/tests/unit/info.tcl +++ b/tests/unit/info.tcl @@ -62,6 +62,19 @@ start_server {tags {"info"}} { assert_equal [s total_error_replies] 2 } + test {errorstats: failed call NOSCRIPT error} { + r config resetstat + assert_equal [s total_error_replies] 0 + assert_match {} [errorstat NOSCRIPT] + catch {r evalsha NotValidShaSUM 0} e + assert_match {NOSCRIPT*} $e + assert_match {*count=1*} [errorstat NOSCRIPT] + assert_match {*calls=1,*,rejected_calls=0,failed_calls=1} [cmdstat evalsha] + assert_equal [s total_error_replies] 1 + r config resetstat + assert_match {} [errorstat NOSCRIPT] + } + test {errorstats: failed call NOGROUP error} { r config resetstat assert_match {} [errorstat NOGROUP] From 5427ba4476b1ab6859614509a6d1d1306120287c Mon Sep 17 00:00:00 2001 From: Madelyn Olson <34459052+madolson@users.noreply.github.com> Date: Wed, 24 Feb 2021 14:26:16 -0800 Subject: [PATCH 11/24] Allow stopped redis processes to be killed in tests (#8552) --- tests/instances.tcl | 2 ++ tests/support/server.tcl | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/instances.tcl b/tests/instances.tcl index 793bce80d..255d9740f 100644 --- a/tests/instances.tcl +++ b/tests/instances.tcl @@ -186,6 +186,8 @@ proc is_alive pid { proc stop_instance pid { catch {exec kill $pid} + # Node might have been stopped in the test + catch {exec kill -SIGCONT $pid} if {$::valgrind} { set max_wait 60000 } else { diff --git a/tests/support/server.tcl b/tests/support/server.tcl index 4fbb99920..3ff923d7e 100644 --- a/tests/support/server.tcl +++ b/tests/support/server.tcl @@ -72,6 +72,8 @@ proc kill_server config { # kill server and wait for the process to be totally exited send_data_packet $::test_server_fd server-killing $pid catch {exec kill $pid} + # Node might have been stopped in the test + catch {exec kill -SIGCONT $pid} if {$::valgrind} { set max_wait 60000 } else { From a21cfa197ab92a2e7033ee0b55170edacc33aeca Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Thu, 25 Feb 2021 09:24:41 +0200 Subject: [PATCH 12/24] Cleanup usage of malloc_usable_size. (#8554) * Add better control of malloc_usable_size() usage. * Use malloc_usable_size on alpine libc daily job. * Add no-malloc-usable-size daily jobs. * Fix zmalloc(0) when HAVE_MALLOC_SIZE is undefined. In order to align with the jemalloc behavior, this should never return NULL or OOM panic. --- .github/workflows/daily.yml | 37 ++++++++++++++++++++++++++++++++++++- src/zmalloc.c | 10 ++++++++-- src/zmalloc.h | 11 +++++++++-- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 9b9d15988..59d236d9a 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -48,6 +48,25 @@ jobs: - name: cluster tests run: ./runtest-cluster + test-ubuntu-no-malloc-usable-size: + runs-on: ubuntu-latest + if: github.repository == 'redis/redis' + timeout-minutes: 14400 + steps: + - uses: actions/checkout@v2 + - name: make + run: make MALLOC=libc CFLAGS=-DNO_MALLOC_USABLE_SIZE + - name: test + run: | + sudo apt-get install tcl8.6 + ./runtest --accurate --verbose --dump-logs + - name: module api test + run: ./runtest-moduleapi --verbose + - name: sentinel tests + run: ./runtest-sentinel + - name: cluster tests + run: ./runtest-cluster + test-ubuntu-32bit: runs-on: ubuntu-latest if: github.repository == 'redis/redis' @@ -132,6 +151,22 @@ jobs: - name: module api test run: ./runtest-moduleapi --valgrind --no-latency --verbose --clients 1 + test-valgrind-no-malloc-usable-size: + runs-on: ubuntu-latest + if: github.repository == 'redis/redis' + timeout-minutes: 14400 + steps: + - uses: actions/checkout@v2 + - name: make + run: make valgrind CFLAGS="-DNO_MALLOC_USABLE_SIZE" + - name: test + run: | + sudo apt-get update + sudo apt-get install tcl8.6 valgrind -y + ./runtest --valgrind --verbose --clients 1 --dump-logs + - name: module api test + run: ./runtest-moduleapi --valgrind --no-latency --verbose --clients 1 + test-centos7-jemalloc: runs-on: ubuntu-latest if: github.repository == 'redis/redis' @@ -250,7 +285,7 @@ jobs: - name: make run: | apk add build-base - make REDIS_CFLAGS='-Werror' USE_JEMALLOC=no + make REDIS_CFLAGS='-Werror' USE_JEMALLOC=no CFLAGS=-DUSE_MALLOC_USABLE_SIZE - name: test run: | apk add tcl procps diff --git a/src/zmalloc.c b/src/zmalloc.c index fbac09616..1dc662e57 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -60,6 +60,11 @@ void zlibc_free(void *ptr) { #define ASSERT_NO_SIZE_OVERFLOW(sz) assert((sz) + PREFIX_SIZE > (sz)) #endif +/* When using the libc allocator, use a minimum allocation size to match the + * jemalloc behavior that doesn't return NULL in this case. + */ +#define MALLOC_MIN_SIZE(x) ((x) > 0 ? (x) : sizeof(long)) + /* Explicitly override malloc/free etc when using tcmalloc. */ #if defined(USE_TCMALLOC) #define malloc(size) tc_malloc(size) @@ -93,7 +98,7 @@ static void (*zmalloc_oom_handler)(size_t) = zmalloc_default_oom; * '*usable' is set to the usable size if non NULL. */ void *ztrymalloc_usable(size_t size, size_t *usable) { ASSERT_NO_SIZE_OVERFLOW(size); - void *ptr = malloc(size+PREFIX_SIZE); + void *ptr = malloc(MALLOC_MIN_SIZE(size)+PREFIX_SIZE); if (!ptr) return NULL; #ifdef HAVE_MALLOC_SIZE @@ -153,7 +158,7 @@ void zfree_no_tcache(void *ptr) { * '*usable' is set to the usable size if non NULL. */ void *ztrycalloc_usable(size_t size, size_t *usable) { ASSERT_NO_SIZE_OVERFLOW(size); - void *ptr = calloc(1, size+PREFIX_SIZE); + void *ptr = calloc(1, MALLOC_MIN_SIZE(size)+PREFIX_SIZE); if (ptr == NULL) return NULL; #ifdef HAVE_MALLOC_SIZE @@ -675,6 +680,7 @@ int zmalloc_test(int argc, char **argv) { UNUSED(argc); UNUSED(argv); + printf("Malloc prefix size: %d\n", (int) PREFIX_SIZE); printf("Initial used memory: %zu\n", zmalloc_used_memory()); ptr = zmalloc(123); printf("Allocated 123 bytes; used: %zu\n", zmalloc_used_memory()); diff --git a/src/zmalloc.h b/src/zmalloc.h index 6f3f6354a..d44c7b389 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -62,11 +62,18 @@ #endif /* On native libc implementations, we should still do our best to provide a - * HAVE_MALLOC_SIZE capability. + * HAVE_MALLOC_SIZE capability. This can be set explicitly as well: + * + * NO_MALLOC_USABLE_SIZE disables it on all platforms, even if they are + * known to support it. + * USE_MALLOC_USABLE_SIZE forces use of malloc_usable_size() regardless + * of platform. */ #ifndef ZMALLOC_LIB #define ZMALLOC_LIB "libc" -#if defined(__GLIBC__) || defined(__FreeBSD__) +#if !defined(NO_MALLOC_USABLE_SIZE) && \ + (defined(__GLIBC__) || defined(__FreeBSD__) || \ + defined(USE_MALLOC_USABLE_SIZE)) #include #define HAVE_MALLOC_SIZE 1 #define zmalloc_size(p) malloc_usable_size(p) From 63bba04778aa698e83eb7f8a06beb3815d4ed46c Mon Sep 17 00:00:00 2001 From: Wen Hui Date: Thu, 25 Feb 2021 13:08:24 -0500 Subject: [PATCH 13/24] redis-cli prompt: fix db number and transaction state inconsistencies after reconnect (#8551) After reconnect, the prompt was showing the db ID and multi state of the previous connection. --- src/redis-cli.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/redis-cli.c b/src/redis-cli.c index ef1d49e3e..3930a2d8e 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -819,6 +819,9 @@ static int cliConnect(int flags) { if (context == NULL || flags & CC_FORCE) { if (context != NULL) { redisFree(context); + config.dbnum = 0; + config.in_multi = 0; + cliRefreshPrompt(); } if (config.hostsocket == NULL) { From 45cd457aa72e2c0ae3473c20cc19b74553a94760 Mon Sep 17 00:00:00 2001 From: Madelyn Olson <34459052+madolson@users.noreply.github.com> Date: Thu, 25 Feb 2021 21:00:27 -0800 Subject: [PATCH 14/24] Moved requirepass and querybuf length to generic configs (#8557) Moved additional configs to generic infrastructure. --- src/acl.c | 13 +++++++- src/config.c | 92 +++++++++------------------------------------------- src/server.c | 4 ++- src/server.h | 2 +- 4 files changed, 31 insertions(+), 80 deletions(-) diff --git a/src/acl.c b/src/acl.c index e120b36fc..445409ecd 100644 --- a/src/acl.c +++ b/src/acl.c @@ -1054,7 +1054,6 @@ void ACLInit(void) { UsersToLoad = listCreate(); ACLLog = listCreate(); ACLInitDefaultUser(); - server.requirepass = NULL; /* Only used for backward compatibility. */ } /* Check the username and password pair and return C_OK if they are valid, @@ -2251,3 +2250,15 @@ void authCommand(client *c) { } } +/* Set the password for the "default" ACL user. This implements supports for + * requirepass config, so passing in NULL will set the user to be nopass. */ +void ACLUpdateDefaultUserPassword(sds password) { + ACLSetUser(DefaultUser,"resetpass",-1); + if (password) { + sds aclop = sdscatlen(sdsnew(">"), password, sdslen(password)); + ACLSetUser(DefaultUser,aclop,sdslen(aclop)); + sdsfree(aclop); + } else { + ACLSetUser(DefaultUser,"nopass",-1); + } +} diff --git a/src/config.c b/src/config.c index 9081d0312..13a6dee14 100644 --- a/src/config.c +++ b/src/config.c @@ -504,8 +504,6 @@ void loadServerConfigFromString(char *config) { } } else if (!strcasecmp(argv[0],"include") && argc == 2) { loadServerConfig(argv[1], 0, NULL); - } else if ((!strcasecmp(argv[0],"client-query-buffer-limit")) && argc == 2) { - server.client_max_querybuf_len = memtoll(argv[1],NULL); } else if ((!strcasecmp(argv[0],"slaveof") || !strcasecmp(argv[0],"replicaof")) && argc == 3) { slaveof_linenum = linenum; @@ -521,26 +519,6 @@ void loadServerConfigFromString(char *config) { err = "Invalid master port"; goto loaderr; } server.repl_state = REPL_STATE_CONNECT; - } else if (!strcasecmp(argv[0],"requirepass") && argc == 2) { - if (sdslen(argv[1]) > CONFIG_AUTHPASS_MAX_LEN) { - err = "Password is longer than CONFIG_AUTHPASS_MAX_LEN"; - goto loaderr; - } - /* The old "requirepass" directive just translates to setting - * a password to the default user. The only thing we do - * additionally is to remember the cleartext password in this - * case, for backward compatibility with Redis <= 5. */ - ACLSetUser(DefaultUser,"resetpass",-1); - sdsfree(server.requirepass); - server.requirepass = NULL; - if (sdslen(argv[1])) { - sds aclop = sdscatlen(sdsnew(">"), argv[1], sdslen(argv[1])); - ACLSetUser(DefaultUser,aclop,sdslen(aclop)); - sdsfree(aclop); - server.requirepass = sdsdup(argv[1]); - } else { - ACLSetUser(DefaultUser,"nopass",-1); - } } else if (!strcasecmp(argv[0],"list-max-ziplist-entries") && argc == 2){ /* DEAD OPTION */ } else if (!strcasecmp(argv[0],"list-max-ziplist-value") && argc == 2) { @@ -750,24 +728,7 @@ void configSetCommand(client *c) { if (0) { /* this starts the config_set macros else-if chain. */ /* Special fields that can't be handled with general macros. */ - config_set_special_field("requirepass") { - if (sdslen(o->ptr) > CONFIG_AUTHPASS_MAX_LEN) goto badfmt; - /* The old "requirepass" directive just translates to setting - * a password to the default user. The only thing we do - * additionally is to remember the cleartext password in this - * case, for backward compatibility with Redis <= 5. */ - ACLSetUser(DefaultUser,"resetpass",-1); - sdsfree(server.requirepass); - server.requirepass = NULL; - if (sdslen(o->ptr)) { - sds aclop = sdscatlen(sdsnew(">"), o->ptr, sdslen(o->ptr)); - ACLSetUser(DefaultUser,aclop,sdslen(aclop)); - sdsfree(aclop); - server.requirepass = sdsdup(o->ptr); - } else { - ACLSetUser(DefaultUser,"nopass",-1); - } - } config_set_special_field("save") { + config_set_special_field("save") { int vlen, j; sds *v = sdssplitlen(o->ptr,sdslen(o->ptr)," ",1,&vlen); @@ -876,10 +837,6 @@ void configSetCommand(client *c) { enableWatchdog(ll); else disableWatchdog(); - /* Memory fields. - * config_set_memory_field(name,var) */ - } config_set_memory_field( - "client-query-buffer-limit",server.client_max_querybuf_len) { /* Everything else is an error... */ } config_set_else { addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s", @@ -959,7 +916,6 @@ void configGetCommand(client *c) { config_get_string_field("logfile",server.logfile); /* Numerical values */ - config_get_numerical_field("client-query-buffer-limit",server.client_max_querybuf_len); config_get_numerical_field("watchdog-period",server.watchdog_period); /* Everything we can't handle with macros follows. */ @@ -1046,16 +1002,6 @@ void configGetCommand(client *c) { sdsfree(aux); matches++; } - if (stringmatch(pattern,"requirepass",1)) { - addReplyBulkCString(c,"requirepass"); - sds password = server.requirepass; - if (password) { - addReplyBulkCBuffer(c,password,sdslen(password)); - } else { - addReplyBulkCString(c,""); - } - matches++; - } if (stringmatch(pattern,"oom-score-adj-values",0)) { sds buf = sdsempty(); @@ -1564,26 +1510,6 @@ void rewriteConfigBindOption(struct rewriteConfigState *state) { rewriteConfigRewriteLine(state,option,line,force); } -/* Rewrite the requirepass option. */ -void rewriteConfigRequirepassOption(struct rewriteConfigState *state, char *option) { - int force = 1; - sds line; - sds password = server.requirepass; - - /* If there is no password set, we don't want the requirepass option - * to be present in the configuration at all. */ - if (password == NULL) { - rewriteConfigMarkAsProcessed(state,option); - return; - } - - line = sdsnew(option); - line = sdscatlen(line, " ", 1); - line = sdscatsds(line, password); - - rewriteConfigRewriteLine(state,option,line,force); -} - /* Glue together the configuration lines in the current configuration * rewrite state into a single string, stripping multiple empty lines. */ sds rewriteConfigGetContentFromState(struct rewriteConfigState *state) { @@ -1740,8 +1666,6 @@ int rewriteConfig(char *path, int force_all) { rewriteConfigUserOption(state); rewriteConfigDirOption(state); rewriteConfigSlaveofOption(state,"replicaof"); - rewriteConfigRequirepassOption(state,"requirepass"); - rewriteConfigBytesOption(state,"client-query-buffer-limit",server.client_max_querybuf_len,PROTO_MAX_QUERYBUF_LEN); rewriteConfigStringOption(state,"cluster-config-file",server.cluster_configfile,CONFIG_DEFAULT_CLUSTER_CONFIG_FILE); rewriteConfigNotifykeyspaceeventsOption(state); rewriteConfigClientoutputbufferlimitOption(state); @@ -2368,6 +2292,18 @@ static int updateOOMScoreAdj(int val, int prev, const char **err) { return 1; } + +int updateRequirePass(sds val, sds prev, const char **err) { + UNUSED(prev); + UNUSED(err); + /* The old "requirepass" directive just translates to setting + * a password to the default user. The only thing we do + * additionally is to remember the cleartext password in this + * case, for backward compatibility with Redis <= 5. */ + ACLUpdateDefaultUserPassword(val); + return 1; +} + #ifdef USE_OPENSSL static int updateTlsCfg(char *val, char *prev, const char **err) { UNUSED(val); @@ -2458,6 +2394,7 @@ standardConfig configs[] = { /* SDS Configs */ createSDSConfig("masterauth", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.masterauth, NULL, NULL, NULL), + createSDSConfig("requirepass", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.requirepass, NULL, NULL, updateRequirePass), /* Enum Configs */ createEnumConfig("supervised", NULL, IMMUTABLE_CONFIG, supervised_mode_enum, server.supervised_mode, SUPERVISED_NONE, NULL, NULL), @@ -2534,6 +2471,7 @@ standardConfig configs[] = { createSizeTConfig("zset-max-ziplist-value", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, server.zset_max_ziplist_value, 64, MEMORY_CONFIG, NULL, NULL), createSizeTConfig("hll-sparse-max-bytes", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, server.hll_sparse_max_bytes, 3000, MEMORY_CONFIG, NULL, NULL), createSizeTConfig("tracking-table-max-keys", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, server.tracking_table_max_keys, 1000000, INTEGER_CONFIG, NULL, NULL), /* Default: 1 million keys max. */ + createSizeTConfig("client-query-buffer-limit", NULL, MODIFIABLE_CONFIG, 1024*1024, LONG_MAX, server.client_max_querybuf_len, 1024*1024*1024, MEMORY_CONFIG, NULL, NULL), /* Default: 1GB max query buffer. */ /* Other configs */ createTimeTConfig("repl-backlog-ttl", NULL, MODIFIABLE_CONFIG, 0, LONG_MAX, server.repl_backlog_time_limit, 60*60, INTEGER_CONFIG, NULL, NULL), /* Default: 1 hour */ diff --git a/src/server.c b/src/server.c index f4dff6b0e..6e9dca3bc 100644 --- a/src/server.c +++ b/src/server.c @@ -2653,7 +2653,6 @@ void initServerConfig(void) { server.sofd = -1; server.active_expire_enabled = 1; server.skip_checksum_validation = 0; - server.client_max_querybuf_len = PROTO_MAX_QUERYBUF_LEN; server.saveparams = NULL; server.loading = 0; server.loading_rdb_used_mem = 0; @@ -3324,6 +3323,9 @@ void initServer(void) { scriptingInit(1); slowlogInit(); latencyMonitorInit(); + + /* Initialize ACL default password if it exists */ + ACLUpdateDefaultUserPassword(server.requirepass); } /* Some steps in server initialization need to be done last (after modules diff --git a/src/server.h b/src/server.h index 55f718111..5bdcaae64 100644 --- a/src/server.h +++ b/src/server.h @@ -138,7 +138,6 @@ typedef long long ustime_t; /* microsecond time type. */ #define STATS_METRIC_COUNT 3 /* Protocol and I/O related defines */ -#define PROTO_MAX_QUERYBUF_LEN (1024*1024*1024) /* 1GB max query buffer. */ #define PROTO_IOBUF_LEN (1024*16) /* Generic I/O buffer size */ #define PROTO_REPLY_CHUNK_BYTES (16*1024) /* 16k output buffer */ #define PROTO_INLINE_MAX_SIZE (1024*64) /* Max size of inline reads */ @@ -2106,6 +2105,7 @@ void addReplyCommandCategories(client *c, struct redisCommand *cmd); user *ACLCreateUnlinkedUser(); void ACLFreeUserAndKillClients(user *u); void addACLLogEntry(client *c, int reason, int keypos, sds username); +void ACLUpdateDefaultUserPassword(sds password); /* Sorted sets data type */ From f24d4002c090b5a620b48b6a95ce78ead2e8cad6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Viktor=20S=C3=B6derqvist?= Date: Sun, 28 Feb 2021 13:11:18 +0100 Subject: [PATCH 15/24] Shared reusable client for RM_Call() (#8516) A single client pointer is added in the server struct. This is initialized by the first RM_Call() and reused for every subsequent RM_Call() except if it's already in use, which means that it's not used for (recursive) module calls to modules. For these, a new "fake" client is created each time. Other changes: * Avoid allocating a dict iterator in pubsubUnsubscribeAllChannels when not needed --- src/module.c | 34 +++++++++++++++++++++++++++++----- src/pubsub.c | 14 ++++++++------ src/server.h | 1 + tests/unit/moduleapi/misc.tcl | 5 +++++ 4 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/module.c b/src/module.c index fe0cd8345..9a37826fb 100644 --- a/src/module.c +++ b/src/module.c @@ -3969,16 +3969,26 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch RedisModuleCallReply *reply = NULL; int replicate = 0; /* Replicate this command? */ - /* Create the client and dispatch the command. */ + /* Handle arguments. */ va_start(ap, fmt); - c = createClient(NULL); - c->user = NULL; /* Root user. */ argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,&flags,ap); replicate = flags & REDISMODULE_ARGV_REPLICATE; va_end(ap); /* Setup our fake client for command execution. */ - c->flags |= CLIENT_MODULE; + if (server.module_client == NULL) { + /* This is the first RM_Call() ever. Create reusable client. */ + c = server.module_client = createClient(NULL); + } else if (server.module_client->argv == NULL) { + /* The reusable module client is not busy with a command. Use it. */ + c = server.module_client; + } else { + /* The reusable module client is busy. (It is probably used in a + * recursive call to this module.) */ + c = createClient(NULL); + } + c->user = NULL; /* Root user. */ + c->flags = CLIENT_MODULE; /* We do not want to allow block, the module do not expect it */ c->flags |= CLIENT_DENY_BLOCKING; @@ -4066,7 +4076,18 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch cleanup: if (ctx->module) ctx->module->in_call--; - freeClient(c); + if (c == server.module_client) { + /* reset shared client so it can be reused */ + discardTransaction(c); + pubsubUnsubscribeAllChannels(c,0); + pubsubUnsubscribeAllPatterns(c,0); + resetClient(c); /* frees the contents of argv */ + zfree(c->argv); + c->argv = NULL; + c->resp = 2; + } else { + freeClient(c); /* temporary client */ + } return reply; } @@ -8269,6 +8290,9 @@ void moduleInitModulesSystem(void) { /* Set up filter list */ moduleCommandFilters = listCreate(); + /* Reusable client for RM_Call() is created on first use */ + server.module_client = NULL; + moduleRegisterCoreAPI(); if (pipe(server.module_blocked_pipe) == -1) { serverLog(LL_WARNING, diff --git a/src/pubsub.c b/src/pubsub.c index a7b370d5d..5f7335bbe 100644 --- a/src/pubsub.c +++ b/src/pubsub.c @@ -250,18 +250,20 @@ int pubsubUnsubscribePattern(client *c, robj *pattern, int notify) { /* Unsubscribe from all the channels. Return the number of channels the * client was subscribed to. */ int pubsubUnsubscribeAllChannels(client *c, int notify) { - dictIterator *di = dictGetSafeIterator(c->pubsub_channels); - dictEntry *de; int count = 0; + if (dictSize(c->pubsub_channels) > 0) { + dictIterator *di = dictGetSafeIterator(c->pubsub_channels); + dictEntry *de; - while((de = dictNext(di)) != NULL) { - robj *channel = dictGetKey(de); + while((de = dictNext(di)) != NULL) { + robj *channel = dictGetKey(de); - count += pubsubUnsubscribeChannel(c,channel,notify); + count += pubsubUnsubscribeChannel(c,channel,notify); + } + dictReleaseIterator(di); } /* We were subscribed to nothing? Still reply to the client. */ if (notify && count == 0) addReplyPubsubUnsubscribed(c,NULL); - dictReleaseIterator(di); return count; } diff --git a/src/server.h b/src/server.h index 5bdcaae64..f52c2ee06 100644 --- a/src/server.h +++ b/src/server.h @@ -1195,6 +1195,7 @@ struct redisServer { to be processed. */ pid_t child_pid; /* PID of current child */ int child_type; /* Type of current child */ + client *module_client; /* "Fake" client to call Redis from modules */ /* Networking */ int port; /* TCP listening port */ int tls_port; /* TLS listening port */ diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl index d60ccca8f..b5cd6100c 100644 --- a/tests/unit/moduleapi/misc.tcl +++ b/tests/unit/moduleapi/misc.tcl @@ -16,6 +16,11 @@ start_server {tags {"modules"}} { assert { [string match "*cmdstat_module*" $info] } } + test {test RM_Call recursive} { + set info [r test.call_generic test.call_generic info commandstats] + assert { [string match "*cmdstat_module*" $info] } + } + test {test redis version} { set version [s redis_version] assert_equal $version [r test.redisversion] From 956949e7a16d53edbe50586b8b7938f99c63e0c1 Mon Sep 17 00:00:00 2001 From: Bonsai Date: Sun, 28 Feb 2021 06:36:37 -0600 Subject: [PATCH 16/24] Module API for getting user name of a client (#8508) Adding RM_GetClientUserNameById to get the ACL user name of a client connection. --- src/module.c | 23 +++++++++++++++++++++++ src/redismodule.h | 2 ++ 2 files changed, 25 insertions(+) diff --git a/src/module.c b/src/module.c index 9a37826fb..274210590 100644 --- a/src/module.c +++ b/src/module.c @@ -1837,6 +1837,28 @@ unsigned long long RM_GetClientId(RedisModuleCtx *ctx) { return ctx->client->id; } +/* Return the ACL user name used by the client with the specified client ID. + * Client ID can be obtained with RM_GetClientId() API. If the client does not + * exist, NULL is returned and errno is set to ENOENT. If the client isn't + * using an ACL user, NULL is returned and errno is set to ENOTSUP */ +RedisModuleString *RM_GetClientUserNameById(RedisModuleCtx *ctx, uint64_t id) { + client *client = lookupClientByID(id); + if (client == NULL) { + errno = ENOENT; + return NULL; + } + + if (client->user == NULL) { + errno = ENOTSUP; + return NULL; + } + + sds name = sdsnew(client->user->name); + robj *str = createObject(OBJ_STRING, name); + autoMemoryAdd(ctx, REDISMODULE_AM_STRING, str); + return str; +} + /* This is an helper for RM_GetClientInfoById() and other functions: given * a client, it populates the client info structure with the appropriate * fields depending on the version provided. If the version is not valid @@ -9157,6 +9179,7 @@ void moduleRegisterCoreAPI(void) { REGISTER_API(IsKeysPositionRequest); REGISTER_API(KeyAtPos); REGISTER_API(GetClientId); + REGISTER_API(GetClientUserNameById); REGISTER_API(GetContextFlags); REGISTER_API(AvoidReplicaTraffic); REGISTER_API(PoolAlloc); diff --git a/src/redismodule.h b/src/redismodule.h index 0c2801bea..ea271b82b 100644 --- a/src/redismodule.h +++ b/src/redismodule.h @@ -665,6 +665,7 @@ REDISMODULE_API long long (*RedisModule_StreamTrimByID)(RedisModuleKey *key, int REDISMODULE_API int (*RedisModule_IsKeysPositionRequest)(RedisModuleCtx *ctx) REDISMODULE_ATTR; REDISMODULE_API void (*RedisModule_KeyAtPos)(RedisModuleCtx *ctx, int pos) REDISMODULE_ATTR; REDISMODULE_API unsigned long long (*RedisModule_GetClientId)(RedisModuleCtx *ctx) REDISMODULE_ATTR; +REDISMODULE_API RedisModuleString * (*RedisModule_GetClientUserNameById)(RedisModuleCtx *ctx, uint64_t id) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_GetClientInfoById)(void *ci, uint64_t id) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_PublishMessage)(RedisModuleCtx *ctx, RedisModuleString *channel, RedisModuleString *message) REDISMODULE_ATTR; REDISMODULE_API int (*RedisModule_GetContextFlags)(RedisModuleCtx *ctx) REDISMODULE_ATTR; @@ -936,6 +937,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int REDISMODULE_GET_API(IsKeysPositionRequest); REDISMODULE_GET_API(KeyAtPos); REDISMODULE_GET_API(GetClientId); + REDISMODULE_GET_API(GetClientUserNameById); REDISMODULE_GET_API(GetContextFlags); REDISMODULE_GET_API(AvoidReplicaTraffic); REDISMODULE_GET_API(PoolAlloc); From ca3b1cd7f5c887602aa3b82ec39d6c9adebe221d Mon Sep 17 00:00:00 2001 From: Pavlo Yatsukhnenko Date: Sun, 28 Feb 2021 15:28:09 +0200 Subject: [PATCH 17/24] Fix div by 0 in redis-cli cluster creation (#8553) This could happen on an invalid use, when trying to create a cluster with a single node and provide it's address 3 time to satisfy redis-cli requirements. --- src/redis-cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 3930a2d8e..63d31f79a 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -2972,7 +2972,7 @@ static void clusterManagerOptimizeAntiAffinity(clusterManagerNodeArray *ipnodes, ip_count, &offenders, &offending_len); - if (score == 0) break; // Optimal anti affinity reached + if (score == 0 || offending_len == 0) break; // Optimal anti affinity reached /* We'll try to randomly swap a slave's assigned master causing * an affinity problem with another random slave, to see if we * can improve the affinity. */ From adfb3a52c6010c2f9cefa673e41eed2bea50186e Mon Sep 17 00:00:00 2001 From: Qu Chen Date: Sun, 28 Feb 2021 21:54:52 -0800 Subject: [PATCH 18/24] Make dbid range check for SWAPDB command consistent with SELECT, MOVE, and COPY. (#8555) DB ID used to be parsed as a long for SWAPDB command, now make it into an int to be consistent with other commands that parses the DB ID argument like SELECT, MOVE, COPY. See #8085 The implication is that the error message when the provided db index is greater than 4M changes slightly. --- src/db.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/db.c b/src/db.c index ed522401f..57705f003 100644 --- a/src/db.c +++ b/src/db.c @@ -1315,7 +1315,7 @@ void scanDatabaseForReadyLists(redisDb *db) { * * Returns C_ERR if at least one of the DB ids are out of range, otherwise * C_OK is returned. */ -int dbSwapDatabases(long id1, long id2) { +int dbSwapDatabases(int id1, int id2) { if (id1 < 0 || id1 >= server.dbnum || id2 < 0 || id2 >= server.dbnum) return C_ERR; if (id1 == id2) return C_OK; @@ -1356,7 +1356,7 @@ int dbSwapDatabases(long id1, long id2) { /* SWAPDB db1 db2 */ void swapdbCommand(client *c) { - long id1, id2; + int id1, id2; /* Not allowed in cluster mode: we have just DB 0 there. */ if (server.cluster_enabled) { @@ -1365,11 +1365,11 @@ void swapdbCommand(client *c) { } /* Get the two DBs indexes. */ - if (getLongFromObjectOrReply(c, c->argv[1], &id1, + if (getIntFromObjectOrReply(c, c->argv[1], &id1, "invalid first DB index") != C_OK) return; - if (getLongFromObjectOrReply(c, c->argv[2], &id2, + if (getIntFromObjectOrReply(c, c->argv[2], &id2, "invalid second DB index") != C_OK) return; From c6fb349f05cb9221ea6302a67b61679047d89ac8 Mon Sep 17 00:00:00 2001 From: David Gilman Date: Mon, 1 Mar 2021 01:15:26 -0500 Subject: [PATCH 19/24] Fix compliation on arm64 Mac with jemalloc (#8458) The arm_thread_state64_get_pc used later in the file is defined in mach kernel headers. Apparently they get included if you use the system malloc but not if you use jemalloc. --- src/debug.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/debug.c b/src/debug.c index a725e5a30..e7fec293a 100644 --- a/src/debug.c +++ b/src/debug.c @@ -54,6 +54,10 @@ typedef ucontext_t sigcontext_t; #endif #endif +#if defined(__APPLE__) && defined(__arm64__) +#include +#endif + /* Globals */ static int bug_report_start = 0; /* True if bug report header was already logged. */ static pthread_mutex_t bug_report_start_mutex = PTHREAD_MUTEX_INITIALIZER; From 34fa7b137e6da6ddb0a215cb577fe9f834f88ca3 Mon Sep 17 00:00:00 2001 From: Bonsai Date: Mon, 1 Mar 2021 00:18:14 -0600 Subject: [PATCH 20/24] fix: call CLIENT INFO from redis module will crash the server (#8560) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because when the RM_Call is invoked. It will create a faker client. The point is client connection is NULL, so server will crash in connGetInfo Co-authored-by: Viktor Söderqvist --- src/connection.c | 2 +- tests/unit/moduleapi/misc.tcl | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/connection.c b/src/connection.c index 3e2021fb5..a59463220 100644 --- a/src/connection.c +++ b/src/connection.c @@ -427,7 +427,7 @@ int connGetState(connection *conn) { * For sockets, we always return "fd=" to maintain compatibility. */ const char *connGetInfo(connection *conn, char *buf, size_t buf_len) { - snprintf(buf, buf_len-1, "fd=%i", conn->fd); + snprintf(buf, buf_len-1, "fd=%i", conn == NULL ? -1 : conn->fd); return buf; } diff --git a/tests/unit/moduleapi/misc.tcl b/tests/unit/moduleapi/misc.tcl index b5cd6100c..a6a7a78f9 100644 --- a/tests/unit/moduleapi/misc.tcl +++ b/tests/unit/moduleapi/misc.tcl @@ -111,4 +111,8 @@ start_server {tags {"modules"}} { r test.log_tsctx "info" "Test message" verify_log_message 0 "* Test message*" 0 } + + test {test RM_Call CLIENT INFO} { + assert_match "*fd=-1*" [r test.call_generic client info] + } } From c0de6fbd29308f118b69adea01fd420bb5893761 Mon Sep 17 00:00:00 2001 From: YaacovHazan <31382944+YaacovHazan@users.noreply.github.com> Date: Mon, 1 Mar 2021 16:04:44 +0200 Subject: [PATCH 21/24] Make port, tls-port and bind configurations modifiable (#8510) Add ability to modify port, tls-port and bind configurations by CONFIG SET command. To simplify the code and make it cleaner, a new structure added, socketFds, which contains the file descriptors array and its counter, and used for TCP, TLS and Cluster sockets file descriptors. --- src/cluster.c | 18 +--- src/config.c | 57 ++++++++++- src/server.c | 180 +++++++++++++++++++++++++++++------ src/server.h | 19 ++-- tests/test_helper.tcl | 1 + tests/unit/introspection.tcl | 4 +- tests/unit/networking.tcl | 34 +++++++ tests/unit/tls.tcl | 25 +++++ 8 files changed, 282 insertions(+), 56 deletions(-) create mode 100644 tests/unit/networking.tcl diff --git a/src/cluster.c b/src/cluster.c index efe2f652d..4605e3ea9 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -499,7 +499,7 @@ void clusterInit(void) { if (saveconf) clusterSaveConfigOrDie(1); /* We need a listening TCP port for our cluster messaging needs. */ - server.cfd_count = 0; + server.cfd.count = 0; /* Port sanity check II * The other handshake port check is triggered too late to stop @@ -512,19 +512,11 @@ void clusterInit(void) { "Your Redis port number must be 55535 or less."); exit(1); } - if (listenToPort(port+CLUSTER_PORT_INCR, - server.cfd,&server.cfd_count) == C_ERR) - { + if (listenToPort(port+CLUSTER_PORT_INCR, &server.cfd) == C_ERR) { exit(1); - } else { - int j; - - for (j = 0; j < server.cfd_count; j++) { - if (aeCreateFileEvent(server.el, server.cfd[j], AE_READABLE, - clusterAcceptHandler, NULL) == AE_ERR) - serverPanic("Unrecoverable error creating Redis Cluster " - "file event."); - } + } + if (createSocketAcceptHandler(&server.cfd, clusterAcceptHandler) != C_OK) { + serverPanic("Unrecoverable error creating Redis Cluster socket accept handler."); } /* The slots -> keys map is a radix tree. Initialize it here. */ diff --git a/src/config.c b/src/config.c index 13a6dee14..1d6ce6b8c 100644 --- a/src/config.c +++ b/src/config.c @@ -728,7 +728,23 @@ void configSetCommand(client *c) { if (0) { /* this starts the config_set macros else-if chain. */ /* Special fields that can't be handled with general macros. */ - config_set_special_field("save") { + config_set_special_field("bind") { + int vlen; + sds *v = sdssplitlen(o->ptr,sdslen(o->ptr)," ",1,&vlen); + + if (vlen < 1 || vlen > CONFIG_BINDADDR_MAX) { + addReplyError(c, "Too many bind addresses specified."); + sdsfreesplitres(v, vlen); + return; + } + + if (changeBindAddr(v, vlen) == C_ERR) { + addReplyError(c, "Failed to bind to specified addresses."); + sdsfreesplitres(v, vlen); + return; + } + sdsfreesplitres(v, vlen); + } config_set_special_field("save") { int vlen, j; sds *v = sdssplitlen(o->ptr,sdslen(o->ptr)," ",1,&vlen); @@ -2191,6 +2207,20 @@ static int updateHZ(long long val, long long prev, const char **err) { return 1; } +static int updatePort(long long val, long long prev, const char **err) { + /* Do nothing if port is unchanged */ + if (val == prev) { + return 1; + } + + if (changeListenPort(val, &server.ipfd, acceptTcpHandler) == C_ERR) { + *err = "Unable to listen on this port. Check server logs."; + return 0; + } + + return 1; +} + static int updateJemallocBgThread(int val, int prev, const char **err) { UNUSED(prev); UNUSED(err); @@ -2329,6 +2359,27 @@ static int updateTlsCfgInt(long long val, long long prev, const char **err) { UNUSED(prev); return updateTlsCfg(NULL, NULL, err); } + +static int updateTLSPort(long long val, long long prev, const char **err) { + /* Do nothing if port is unchanged */ + if (val == prev) { + return 1; + } + + /* Configure TLS if tls is enabled */ + if (prev == 0 && tlsConfigure(&server.tls_ctx_config) == C_ERR) { + *err = "Unable to update TLS configuration. Check server logs."; + return 0; + } + + if (changeListenPort(val, &server.tlsfd, acceptTLSHandler) == C_ERR) { + *err = "Unable to listen on this port. Check server logs."; + return 0; + } + + return 1; +} + #endif /* USE_OPENSSL */ standardConfig configs[] = { @@ -2409,7 +2460,7 @@ standardConfig configs[] = { /* Integer configs */ createIntConfig("databases", NULL, IMMUTABLE_CONFIG, 1, INT_MAX, server.dbnum, 16, INTEGER_CONFIG, NULL, NULL), - createIntConfig("port", NULL, IMMUTABLE_CONFIG, 0, 65535, server.port, 6379, INTEGER_CONFIG, NULL, NULL), /* TCP port. */ + createIntConfig("port", NULL, MODIFIABLE_CONFIG, 0, 65535, server.port, 6379, INTEGER_CONFIG, NULL, updatePort), /* TCP port. */ createIntConfig("io-threads", NULL, IMMUTABLE_CONFIG, 1, 128, server.io_threads_num, 1, INTEGER_CONFIG, NULL, NULL), /* Single threaded by default */ createIntConfig("auto-aof-rewrite-percentage", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.aof_rewrite_perc, 100, INTEGER_CONFIG, NULL, NULL), createIntConfig("cluster-replica-validity-factor", "cluster-slave-validity-factor", MODIFIABLE_CONFIG, 0, INT_MAX, server.cluster_slave_validity_factor, 10, INTEGER_CONFIG, NULL, NULL), /* Slave max data age factor. */ @@ -2478,7 +2529,7 @@ standardConfig configs[] = { createOffTConfig("auto-aof-rewrite-min-size", NULL, MODIFIABLE_CONFIG, 0, LLONG_MAX, server.aof_rewrite_min_size, 64*1024*1024, MEMORY_CONFIG, NULL, NULL), #ifdef USE_OPENSSL - createIntConfig("tls-port", NULL, IMMUTABLE_CONFIG, 0, 65535, server.tls_port, 0, INTEGER_CONFIG, NULL, updateTlsCfgInt), /* TCP port. */ + createIntConfig("tls-port", NULL, MODIFIABLE_CONFIG, 0, 65535, server.tls_port, 0, INTEGER_CONFIG, NULL, updateTLSPort), /* TCP port. */ createIntConfig("tls-session-cache-size", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.tls_ctx_config.session_cache_size, 20*1024, INTEGER_CONFIG, NULL, updateTlsCfgInt), createIntConfig("tls-session-cache-timeout", NULL, MODIFIABLE_CONFIG, 0, INT_MAX, server.tls_ctx_config.session_cache_timeout, 300, INTEGER_CONFIG, NULL, updateTlsCfgInt), createBoolConfig("tls-cluster", NULL, MODIFIABLE_CONFIG, server.tls_cluster, 0, NULL, updateTlsCfgBool), diff --git a/src/server.c b/src/server.c index 6e9dca3bc..4784431ba 100644 --- a/src/server.c +++ b/src/server.c @@ -2648,8 +2648,8 @@ void initServerConfig(void) { server.arch_bits = (sizeof(long) == 8) ? 64 : 32; server.bindaddr_count = 0; server.unixsocketperm = CONFIG_DEFAULT_UNIX_SOCKET_PERM; - server.ipfd_count = 0; - server.tlsfd_count = 0; + server.ipfd.count = 0; + server.tlsfd.count = 0; server.sofd = -1; server.active_expire_enabled = 1; server.skip_checksum_validation = 0; @@ -2988,6 +2988,34 @@ void checkTcpBacklogSettings(void) { #endif } +void closeSocketListeners(socketFds *sfd) { + int j; + + for (j = 0; j < sfd->count; j++) { + if (sfd->fd[j] == -1) continue; + + aeDeleteFileEvent(server.el, sfd->fd[j], AE_READABLE); + close(sfd->fd[j]); + } + + sfd->count = 0; +} + +/* Create an event handler for accepting new connections in TCP or TLS domain sockets. + * This works atomically for all socket fds */ +int createSocketAcceptHandler(socketFds *sfd, aeFileProc *accept_handler) { + int j; + + for (j = 0; j < sfd->count; j++) { + if (aeCreateFileEvent(server.el, sfd->fd[j], AE_READABLE, accept_handler,NULL) == AE_ERR) { + /* Rollback */ + for (j = j-1; j >= 0; j--) aeDeleteFileEvent(server.el, sfd->fd[j], AE_READABLE); + return C_ERR; + } + } + return C_OK; +} + /* Initialize a set of file descriptors to listen to the specified 'port' * binding the addresses specified in the Redis server configuration. * @@ -3006,7 +3034,7 @@ void checkTcpBacklogSettings(void) { * impossible to bind, or no bind addresses were specified in the server * configuration but the function is not able to bind * for at least * one of the IPv4 or IPv6 protocols. */ -int listenToPort(int port, int *fds, int *count) { +int listenToPort(int port, socketFds *sfd) { int j; char **bindaddr = server.bindaddr; int bindaddr_count = server.bindaddr_count; @@ -3024,12 +3052,12 @@ int listenToPort(int port, int *fds, int *count) { if (optional) addr++; if (strchr(addr,':')) { /* Bind IPv6 address. */ - fds[*count] = anetTcp6Server(server.neterr,port,addr,server.tcp_backlog); + sfd->fd[sfd->count] = anetTcp6Server(server.neterr,port,addr,server.tcp_backlog); } else { /* Bind IPv4 address. */ - fds[*count] = anetTcpServer(server.neterr,port,addr,server.tcp_backlog); + sfd->fd[sfd->count] = anetTcpServer(server.neterr,port,addr,server.tcp_backlog); } - if (fds[*count] == ANET_ERR) { + if (sfd->fd[sfd->count] == ANET_ERR) { serverLog(LL_WARNING, "Could not create server TCP listening socket %s:%d: %s", addr, port, server.neterr); @@ -3039,11 +3067,14 @@ int listenToPort(int port, int *fds, int *count) { errno == ESOCKTNOSUPPORT || errno == EPFNOSUPPORT || errno == EAFNOSUPPORT) continue; + + /* Rollback successful listens before exiting */ + closeSocketListeners(sfd); return C_ERR; } - anetNonBlock(NULL,fds[*count]); - anetCloexec(fds[*count]); - (*count)++; + anetNonBlock(NULL,sfd->fd[sfd->count]); + anetCloexec(sfd->fd[sfd->count]); + sfd->count++; } return C_OK; } @@ -3165,10 +3196,10 @@ void initServer(void) { /* Open the TCP listening socket for the user commands. */ if (server.port != 0 && - listenToPort(server.port,server.ipfd,&server.ipfd_count) == C_ERR) + listenToPort(server.port,&server.ipfd) == C_ERR) exit(1); if (server.tls_port != 0 && - listenToPort(server.tls_port,server.tlsfd,&server.tlsfd_count) == C_ERR) + listenToPort(server.tls_port,&server.tlsfd) == C_ERR) exit(1); /* Open the listening Unix domain socket. */ @@ -3185,7 +3216,7 @@ void initServer(void) { } /* Abort if there are no listening sockets at all. */ - if (server.ipfd_count == 0 && server.tlsfd_count == 0 && server.sofd < 0) { + if (server.ipfd.count == 0 && server.tlsfd.count == 0 && server.sofd < 0) { serverLog(LL_WARNING, "Configured to not listen anywhere, exiting."); exit(1); } @@ -3263,21 +3294,11 @@ void initServer(void) { /* Create an event handler for accepting new connections in TCP and Unix * domain sockets. */ - for (j = 0; j < server.ipfd_count; j++) { - if (aeCreateFileEvent(server.el, server.ipfd[j], AE_READABLE, - acceptTcpHandler,NULL) == AE_ERR) - { - serverPanic( - "Unrecoverable error creating server.ipfd file event."); - } + if (createSocketAcceptHandler(&server.ipfd, acceptTcpHandler) != C_OK) { + serverPanic("Unrecoverable error creating TCP socket accept handler."); } - for (j = 0; j < server.tlsfd_count; j++) { - if (aeCreateFileEvent(server.el, server.tlsfd[j], AE_READABLE, - acceptTLSHandler,NULL) == AE_ERR) - { - serverPanic( - "Unrecoverable error creating server.tlsfd file event."); - } + if (createSocketAcceptHandler(&server.tlsfd, acceptTLSHandler) != C_OK) { + serverPanic("Unrecoverable error creating TLS socket accept handler."); } if (server.sofd > 0 && aeCreateFileEvent(server.el,server.sofd,AE_READABLE, acceptUnixHandler,NULL) == AE_ERR) serverPanic("Unrecoverable error creating server.sofd file event."); @@ -4182,11 +4203,11 @@ void incrementErrorCount(const char *fullerr, size_t namelen) { void closeListeningSockets(int unlink_unix_socket) { int j; - for (j = 0; j < server.ipfd_count; j++) close(server.ipfd[j]); - for (j = 0; j < server.tlsfd_count; j++) close(server.tlsfd[j]); + for (j = 0; j < server.ipfd.count; j++) close(server.ipfd.fd[j]); + for (j = 0; j < server.tlsfd.count; j++) close(server.tlsfd.fd[j]); if (server.sofd != -1) close(server.sofd); if (server.cluster_enabled) - for (j = 0; j < server.cfd_count; j++) close(server.cfd[j]); + for (j = 0; j < server.cfd.count; j++) close(server.cfd.fd[j]); if (unlink_unix_socket && server.unixsocket) { serverLog(LL_NOTICE,"Removing the unix socket file."); unlink(server.unixsocket); /* don't care if this fails */ @@ -5536,6 +5557,105 @@ void redisAsciiArt(void) { zfree(buf); } +int changeBindAddr(sds *addrlist, int addrlist_len) { + int i; + int result = C_OK; + + char *prev_bindaddr[CONFIG_BINDADDR_MAX]; + int prev_bindaddr_count; + + /* Close old TCP and TLS servers */ + closeSocketListeners(&server.ipfd); + closeSocketListeners(&server.tlsfd); + + /* Keep previous settings */ + prev_bindaddr_count = server.bindaddr_count; + memcpy(prev_bindaddr, server.bindaddr, sizeof(server.bindaddr)); + + /* Copy new settings */ + memset(server.bindaddr, 0, sizeof(server.bindaddr)); + for (i = 0; i < addrlist_len; i++) { + server.bindaddr[i] = zstrdup(addrlist[i]); + } + server.bindaddr_count = addrlist_len; + + /* Bind to the new port */ + if ((server.port != 0 && listenToPort(server.port, &server.ipfd) != C_OK) || + (server.tls_port != 0 && listenToPort(server.tls_port, &server.tlsfd) != C_OK)) { + serverLog(LL_WARNING, "Failed to bind, trying to restore old listening sockets."); + + /* Restore old bind addresses */ + for (i = 0; i < addrlist_len; i++) { + zfree(server.bindaddr[i]); + } + memcpy(server.bindaddr, prev_bindaddr, sizeof(server.bindaddr)); + server.bindaddr_count = prev_bindaddr_count; + + /* Re-Listen TCP and TLS */ + server.ipfd.count = 0; + if (server.port != 0 && listenToPort(server.port, &server.ipfd) != C_OK) { + serverPanic("Failed to restore old listening sockets."); + } + + server.tlsfd.count = 0; + if (server.tls_port != 0 && listenToPort(server.tls_port, &server.tlsfd) != C_OK) { + serverPanic("Failed to restore old listening sockets."); + } + + result = C_ERR; + } else { + /* Free old bind addresses */ + for (i = 0; i < prev_bindaddr_count; i++) { + zfree(prev_bindaddr[i]); + } + } + + /* Create TCP and TLS event handlers */ + if (createSocketAcceptHandler(&server.ipfd, acceptTcpHandler) != C_OK) { + serverPanic("Unrecoverable error creating TCP socket accept handler."); + } + if (createSocketAcceptHandler(&server.tlsfd, acceptTLSHandler) != C_OK) { + serverPanic("Unrecoverable error creating TLS socket accept handler."); + } + + if (server.set_proc_title) redisSetProcTitle(NULL); + + return result; +} + +int changeListenPort(int port, socketFds *sfd, aeFileProc *accept_handler) { + socketFds new_sfd = {{0}}; + + /* Just close the server if port disabled */ + if (port == 0) { + closeSocketListeners(sfd); + if (server.set_proc_title) redisSetProcTitle(NULL); + return C_OK; + } + + /* Bind to the new port */ + if (listenToPort(port, &new_sfd) != C_OK) { + return C_ERR; + } + + /* Create event handlers */ + if (createSocketAcceptHandler(&new_sfd, accept_handler) != C_OK) { + closeSocketListeners(&new_sfd); + return C_ERR; + } + + /* Close old servers */ + closeSocketListeners(sfd); + + /* Copy new descriptors */ + sfd->count = new_sfd.count; + memcpy(sfd->fd, new_sfd.fd, sizeof(new_sfd.fd)); + + if (server.set_proc_title) redisSetProcTitle(NULL); + + return C_OK; +} + static void sigShutdownHandler(int sig) { char *msg; @@ -6124,7 +6244,7 @@ int main(int argc, char **argv) { exit(1); } } - if (server.ipfd_count > 0 || server.tlsfd_count > 0) + if (server.ipfd.count > 0 || server.tlsfd.count > 0) serverLog(LL_NOTICE,"Ready to accept connections"); if (server.sofd > 0) serverLog(LL_NOTICE,"The server is now ready to accept connections at %s", server.unixsocket); diff --git a/src/server.h b/src/server.h index f52c2ee06..e241bad70 100644 --- a/src/server.h +++ b/src/server.h @@ -1104,6 +1104,11 @@ struct malloc_stats { size_t allocator_resident; }; +typedef struct socketFds { + int fd[CONFIG_BINDADDR_MAX]; + int count; +} socketFds; + /*----------------------------------------------------------------------------- * TLS Context Configuration *----------------------------------------------------------------------------*/ @@ -1204,13 +1209,10 @@ struct redisServer { int bindaddr_count; /* Number of addresses in server.bindaddr[] */ char *unixsocket; /* UNIX socket path */ mode_t unixsocketperm; /* UNIX socket permission */ - int ipfd[CONFIG_BINDADDR_MAX]; /* TCP socket file descriptors */ - int ipfd_count; /* Used slots in ipfd[] */ - int tlsfd[CONFIG_BINDADDR_MAX]; /* TLS socket file descriptors */ - int tlsfd_count; /* Used slots in tlsfd[] */ + socketFds ipfd; /* TCP socket file descriptors */ + socketFds tlsfd; /* TLS socket file descriptors */ int sofd; /* Unix socket file descriptor */ - int cfd[CONFIG_BINDADDR_MAX];/* Cluster bus listening socket */ - int cfd_count; /* Used slots in cfd[] */ + socketFds cfd; /* Cluster bus listening socket */ list *clients; /* List of active clients */ list *clients_to_close; /* Clients to close asynchronously */ list *clients_pending_write; /* There is to write or install handler. */ @@ -1855,7 +1857,7 @@ int getClientTypeByName(char *name); char *getClientTypeName(int class); void flushSlavesOutputBuffers(void); void disconnectSlaves(void); -int listenToPort(int port, int *fds, int *count); +int listenToPort(int port, socketFds *fds); void pauseClients(mstime_t duration, pause_type type); void unpauseClients(void); int areClientsPaused(void); @@ -2184,6 +2186,9 @@ int processCommand(client *c); int processPendingCommandsAndResetClient(client *c); void setupSignalHandlers(void); void removeSignalHandlers(void); +int createSocketAcceptHandler(socketFds *sfd, aeFileProc *accept_handler); +int changeListenPort(int port, socketFds *sfd, aeFileProc *accept_handler); +int changeBindAddr(sds *addrlist, int addrlist_len); struct redisCommand *lookupCommand(sds name); struct redisCommand *lookupCommandByCString(const char *s); struct redisCommand *lookupCommandOrOriginal(sds name); diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index eb14b5ee9..3c7d46d57 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -75,6 +75,7 @@ set ::all_tests { unit/tracking unit/oom-score-adj unit/shutdown + unit/networking } # Index to the next test to run in the ::all_tests list. set ::next_test 0 diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index c1cf72f0c..698ed7789 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -100,13 +100,10 @@ start_server {tags {"introspection"}} { supervised syslog-facility databases - port - tls-port io-threads logfile unixsocketperm slaveof - bind requirepass server_cpulist bio_cpulist @@ -131,6 +128,7 @@ start_server {tags {"introspection"}} { tls-protocols tls-ciphers tls-ciphersuites + tls-port } } diff --git a/tests/unit/networking.tcl b/tests/unit/networking.tcl new file mode 100644 index 000000000..1b5f19709 --- /dev/null +++ b/tests/unit/networking.tcl @@ -0,0 +1,34 @@ +test {CONFIG SET port number} { + start_server {} { + # available port + set avail_port [find_available_port $::baseport $::portcount] + set rd [redis [srv 0 host] [srv 0 port] 0 0] + $rd CONFIG SET port $avail_port + $rd close + set rd [redis [srv 0 host] $avail_port 0 0] + $rd PING + + # already inuse port + catch {$rd CONFIG SET port $::test_server_port} e + assert_match {*Unable to listen on this port*} $e + $rd close + + # make sure server still listening on the previous port + set rd [redis [srv 0 host] $avail_port 0 0] + $rd PING + $rd close + } +} + +test {CONFIG SET bind address} { + start_server {} { + # non-valid address + catch {r CONFIG SET bind "some.wrong.bind.address"} e + assert_match {*Failed to bind to specified addresses*} $e + + # make sure server still bound to the previous address + set rd [redis [srv 0 host] [srv 0 port] 0 0] + $rd PING + $rd close + } +} \ No newline at end of file diff --git a/tests/unit/tls.tcl b/tests/unit/tls.tcl index a6c2f3c2c..14e06fcdf 100644 --- a/tests/unit/tls.tcl +++ b/tests/unit/tls.tcl @@ -114,5 +114,30 @@ start_server {tags {"tls"}} { } } } + + test {TLS: switch between tcp and tls ports} { + set srv_port [srv 0 port] + + # TLS + set rd [redis [srv 0 host] $srv_port 0 1] + $rd PING + + # TCP + $rd CONFIG SET tls-port 0 + $rd CONFIG SET port $srv_port + $rd close + + set rd [redis [srv 0 host] $srv_port 0 0] + $rd PING + + # TLS + $rd CONFIG SET port 0 + $rd CONFIG SET tls-port $srv_port + $rd close + + set rd [redis [srv 0 host] $srv_port 0 1] + $rd PING + $rd close + } } } From e978fc37edbfdefc49b1b2607073af889c85e34c Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Mon, 1 Mar 2021 17:23:29 +0200 Subject: [PATCH 22/24] fix stream deep sanitization with deleted records (#8568) When sanitizing the stream listpack, we need to count the deleted records too. otherwise the last line that checks the next pointer fails. Add test to cover that state in the stream tests. --- src/t_stream.c | 3 ++- tests/integration/corrupt-dump-fuzzer.tcl | 1 + tests/integration/rdb.tcl | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/t_stream.c b/src/t_stream.c index 4a425fd78..18138bec5 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -3570,7 +3570,7 @@ int streamValidateListpackIntegrity(unsigned char *lp, size_t size, int deep) { p = next; if (!lpValidateNext(lp, &next, size)) return 0; /* deleted */ - lpGetIntegerIfValid(p, &valid_record); + int64_t deleted_count = lpGetIntegerIfValid(p, &valid_record); if (!valid_record) return 0; p = next; if (!lpValidateNext(lp, &next, size)) return 0; @@ -3589,6 +3589,7 @@ int streamValidateListpackIntegrity(unsigned char *lp, size_t size, int deep) { if (!valid_record || zero != 0) return 0; p = next; if (!lpValidateNext(lp, &next, size)) return 0; + entry_count += deleted_count; while (entry_count--) { if (!p) return 0; int64_t fields = master_fields, extra_fields = 3; diff --git a/tests/integration/corrupt-dump-fuzzer.tcl b/tests/integration/corrupt-dump-fuzzer.tcl index 2506205a5..4fb503b8e 100644 --- a/tests/integration/corrupt-dump-fuzzer.tcl +++ b/tests/integration/corrupt-dump-fuzzer.tcl @@ -32,6 +32,7 @@ proc generate_types {} { # add some metadata to the stream r xgroup create stream mygroup 0 set records [r xreadgroup GROUP mygroup Alice COUNT 2 STREAMS stream >] + r xdel stream [lindex [lindex [lindex [lindex $records 0] 1] 1] 0] r xack stream mygroup [lindex [lindex [lindex [lindex $records 0] 1] 0] 0] # create other non-collection types diff --git a/tests/integration/rdb.tcl b/tests/integration/rdb.tcl index 7df1e2f74..9e1c2651a 100644 --- a/tests/integration/rdb.tcl +++ b/tests/integration/rdb.tcl @@ -52,6 +52,7 @@ start_server [list overrides [list "dir" $server_path] keep_persistence true] { } r xgroup create stream mygroup 0 set records [r xreadgroup GROUP mygroup Alice COUNT 2 STREAMS stream >] + r xdel stream [lindex [lindex [lindex [lindex $records 0] 1] 1] 0] r xack stream mygroup [lindex [lindex [lindex [lindex $records 0] 1] 0] 0] set digest [r debug digest] r config set sanitize-dump-payload no From 50fbcbb4df660fdaa03478f5f3952feb3f3f5b0f Mon Sep 17 00:00:00 2001 From: YaacovHazan <31382944+YaacovHazan@users.noreply.github.com> Date: Mon, 1 Mar 2021 20:53:02 +0200 Subject: [PATCH 23/24] fix new networking tests to work when the test suite is used in tls mode (#8582) the tests were unable to connect to the server since the attempted to use normal tcp --- tests/unit/networking.tcl | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/unit/networking.tcl b/tests/unit/networking.tcl index 1b5f19709..19feee8c3 100644 --- a/tests/unit/networking.tcl +++ b/tests/unit/networking.tcl @@ -1,20 +1,22 @@ test {CONFIG SET port number} { start_server {} { + if {$::tls} { set port_cfg tls-port} else { set port_cfg port } + # available port set avail_port [find_available_port $::baseport $::portcount] - set rd [redis [srv 0 host] [srv 0 port] 0 0] - $rd CONFIG SET port $avail_port + set rd [redis [srv 0 host] [srv 0 port] 0 $::tls] + $rd CONFIG SET $port_cfg $avail_port $rd close - set rd [redis [srv 0 host] $avail_port 0 0] + set rd [redis [srv 0 host] $avail_port 0 $::tls] $rd PING # already inuse port - catch {$rd CONFIG SET port $::test_server_port} e + catch {$rd CONFIG SET $port_cfg $::test_server_port} e assert_match {*Unable to listen on this port*} $e $rd close # make sure server still listening on the previous port - set rd [redis [srv 0 host] $avail_port 0 0] + set rd [redis [srv 0 host] $avail_port 0 $::tls] $rd PING $rd close } @@ -27,7 +29,7 @@ test {CONFIG SET bind address} { assert_match {*Failed to bind to specified addresses*} $e # make sure server still bound to the previous address - set rd [redis [srv 0 host] [srv 0 port] 0 0] + set rd [redis [srv 0 host] [srv 0 port] 0 $::tls] $rd PING $rd close } From 003cff7b0a2b7cc4fa5b95ffd4c0639b9bde673a Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Mon, 1 Mar 2021 21:02:05 +0200 Subject: [PATCH 24/24] Redis 6.2.1 --- 00-RELEASENOTES | 27 +++++++++++++++++++++++++++ src/version.h | 4 ++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/00-RELEASENOTES b/00-RELEASENOTES index 36ef7156f..d94c60465 100644 --- a/00-RELEASENOTES +++ b/00-RELEASENOTES @@ -11,6 +11,33 @@ CRITICAL: There is a critical bug affecting MOST USERS. Upgrade ASAP. SECURITY: There are security fixes in the release. -------------------------------------------------------------------------------- +================================================================================ +Redis 6.2.1 Released Mon Mar 1 17:51:36 IST 2021 +================================================================================ + +Upgrade urgency: LOW. + +Here is a comprehensive list of changes in this release compared to 6.2.0, +each one includes the PR number that added it, so you can get more details +at https://github.com/redis/redis/pull/ + +Bug fixes: +* Fix sanitize-dump-payload for stream with deleted records (#8568) +* Prevent client-query-buffer-limit config from being set to lower than 1mb (#8557) + +Improvements: +* Make port, tls-port and bind config options modifiable at runtime (#8510) + +Platform and deployment-related changes: +* Fix compilation error on non-glibc systems if jemalloc is not used (#8533) +* Improved memory consumption and memory usage tracking on FreeBSD (#8545) +* Fix compilation on ARM64 MacOS with jemalloc (#8458) + +Modules: +* New Module API for getting user name of a client (#8508) +* Optimize RM_Call by utilizing a shared reusable client (#8516) +* Fix crash running CLIENT INFO via RM_Call (#8560) + ================================================================================ Redis 6.2.0 GA Released Tue Feb 22 14:00:00 IST 2021 ================================================================================ diff --git a/src/version.h b/src/version.h index 62ddcfd59..64fdbe89b 100644 --- a/src/version.h +++ b/src/version.h @@ -1,2 +1,2 @@ -#define REDIS_VERSION "6.2.0" -#define REDIS_VERSION_NUM 0x00060200 +#define REDIS_VERSION "6.2.1" +#define REDIS_VERSION_NUM 0x00060201