diff --git a/src/Makefile b/src/Makefile index 9d8125609b3..a91fd52da7e 100644 --- a/src/Makefile +++ b/src/Makefile @@ -570,7 +570,7 @@ test: $(REDIS_SERVER_NAME) $(REDIS_CHECK_AOF_NAME) $(REDIS_CLI_NAME) $(REDIS_BEN @(cd ..; ./runtest) test-asan: $(REDIS_SERVER_NAME) $(REDIS_CHECK_AOF_NAME) $(REDIS_CLI_NAME) $(REDIS_BENCHMARK_NAME) - @(cd ..; ./runtest --tags -nosanitizer) + @(cd ..; ./runtest --tags -nosanitizer --asan) test-modules: $(REDIS_SERVER_NAME) @(cd ..; ./runtest-moduleapi) diff --git a/src/debug.c b/src/debug.c index 88af0a22dc6..4a43d01de86 100644 --- a/src/debug.c +++ b/src/debug.c @@ -2288,6 +2288,14 @@ int memtest_test_linux_anonymous_maps(void) { end_addr = strtoul(end,NULL,16); size = end_addr-start_addr; + if (regions >= MEMTEST_MAX_REGIONS) { + snprintf(logbuf,sizeof(logbuf), + "*** Too many memory regions (max %d), skipping remaining regions\n", + MEMTEST_MAX_REGIONS); + if (write(fd,logbuf,strlen(logbuf)) == -1) { /* Nothing to do. */ } + break; + } + start_vect[regions] = start_addr; size_vect[regions] = size; snprintf(logbuf,sizeof(logbuf), @@ -2338,6 +2346,24 @@ void killThreads(void) { void doFastMemoryTest(void) { #if defined(HAVE_PROC_MAPS) +#if defined(__has_feature) +# if __has_feature(address_sanitizer) +# define RUNNING_ASAN 1 +# endif +#endif +#ifdef __SANITIZE_ADDRESS__ +# define RUNNING_ASAN 1 +#endif + +#ifdef RUNNING_ASAN + /* Skip memory test when running under AddressSanitizer as it will + * interfere with ASAN's shadow memory and cause false positives. */ + if (server.memcheck_enabled) { + serverLogRaw(LL_WARNING|LL_RAW, "\n------ FAST MEMORY TEST ------\n"); + serverLogRaw(LL_WARNING|LL_RAW, + "Memory test skipped: running under AddressSanitizer\n"); + } +#else if (server.memcheck_enabled) { /* Test memory */ serverLogRaw(LL_WARNING|LL_RAW, "\n------ FAST MEMORY TEST ------\n"); @@ -2350,6 +2376,7 @@ void doFastMemoryTest(void) { "Fast memory test PASSED, however your memory can still be broken. Please run a memory test for several hours if possible.\n"); } } +#endif #endif /* HAVE_PROC_MAPS */ } diff --git a/src/networking.c b/src/networking.c index 907687bf063..b7acad84bf2 100644 --- a/src/networking.c +++ b/src/networking.c @@ -1793,6 +1793,7 @@ static void resetReusableQueryBuf(client *c) { static void deferFreeClient(client *c) { sds client_desc; serverAssert(c->keyrequests_count); + if (c->CLIENT_DEFERED_CLOSING) return; client_desc = catClientInfoString(sdsempty(), c); serverLog(LL_NOTICE, "Defer client close: %s", client_desc); @@ -1810,17 +1811,25 @@ void freeClientsInDeferedQueue(void) { listIter li; listNode *ln; + /* We can safely use listIter here because listNext() saves the next node + * pointer BEFORE returning current node, so deleting current node doesn't + * invalidate the iterator. However, we must delete the node BEFORE calling + * freeClient() to avoid use-after-free if freeClient() recurses. */ listRewind(server.clients_to_free, &li); while ((ln = listNext(&li))) { client *c = listNodeValue(ln); if (!c->keyrequests_count) { client_desc = catClientInfoString(sdsempty(), c); c->CLIENT_DEFERED_CLOSING = 0; + /* IMPORTANT: Remove from list BEFORE freeing to avoid: + * 1. Use-after-free if freeClient() recurses back here + * 2. Iterator pointing to freed memory */ + listDelNode(server.clients_to_free, ln); freeClient(c); serverLog(LL_NOTICE, "Defered client closed: %s", client_desc); sdsfree(client_desc); - listDelNode(server.clients_to_free,ln); } + /* If client has pending key requests, skip it and continue with next client */ } } diff --git a/src/server.c b/src/server.c index 58795faf3eb..28ea1454235 100644 --- a/src/server.c +++ b/src/server.c @@ -1067,8 +1067,17 @@ int updateClientMemUsageAndBucket(client *c) { * running_tid is the main thread. The true main thread is allowed to call * this function on clients handled by IO-threads as it makes sure the * IO-threads are paused, f.e see cleintsCron() and evictClients(). */ +#ifdef ENABLE_SWAP + if (!((pthread_equal(pthread_self(), server.main_thread_id) || + c->running_tid == IOTHREAD_MAIN_THREAD_ID) && c->conn)) { + return 0; + } + +#else serverAssert((pthread_equal(pthread_self(), server.main_thread_id) || c->running_tid == IOTHREAD_MAIN_THREAD_ID) && c->conn); +#endif + int allow_eviction = clientEvictionAllowed(c); removeClientFromMemUsageBucket(c, allow_eviction); diff --git a/tests/integration/failover.tcl b/tests/integration/failover.tcl index bd33f84aba6..4b623803040 100644 --- a/tests/integration/failover.tcl +++ b/tests/integration/failover.tcl @@ -1,4 +1,10 @@ start_server {tags {"failover external:skip"} overrides {save {}}} { + if {$::asan} { + # ASAN needs more time to complete the failover + set failover_timeout 200 + } else { + set failover_timeout 50 + } start_server {overrides {save {}}} { start_server {overrides {save {}}} { set node_0 [srv 0 client] @@ -83,8 +89,8 @@ start_server {overrides {save {}}} { # Execute the failover $node_0 failover to $node_1_host $node_1_port - # Wait for failover to end - wait_for_condition 50 100 { + # Wait for failover to end (increased timeout for ASAN compatibility) + wait_for_condition $failover_timeout 100 { [s 0 master_failover_state] == "no-failover" } else { fail "Failover from node 0 to node 1 did not finish" @@ -118,8 +124,8 @@ start_server {overrides {save {}}} { $node_1 set CASE 1 $node_1 FAILOVER - # Wait for failover to end - wait_for_condition 50 100 { + # Wait for failover to end (increased timeout for ASAN compatibility) + wait_for_condition $failover_timeout 100 { [s -1 master_failover_state] == "no-failover" } else { fail "Failover from node 1 to node 2 did not finish" @@ -149,8 +155,8 @@ start_server {overrides {save {}}} { $node_2 set case 2 $node_2 failover to $node_0_host $node_0_port TIMEOUT 100 FORCE - # Wait for node 0 to give up on sync attempt and start failover - wait_for_condition 50 100 { + # Wait for node 0 to give up on sync attempt and start failover (increased timeout for ASAN) + wait_for_condition $failover_timeout 100 { [s -2 master_failover_state] == "failover-in-progress" } else { fail "Failover from node 2 to node 0 did not timeout" @@ -163,8 +169,8 @@ start_server {overrides {save {}}} { resume_process $node_0_pid - # Wait for failover to end - wait_for_condition 50 100 { + # Wait for failover to end (increased timeout for ASAN compatibility) + wait_for_condition $failover_timeout 100 { [s -2 master_failover_state] == "no-failover" } else { fail "Failover from node 2 to node 0 did not finish" @@ -267,8 +273,8 @@ start_server {overrides {save {}}} { $node_0 failover to $node_1_host $node_1_port resume_process [srv -1 pid] - # Wait for failover to end - wait_for_condition 50 100 { + # Wait for failover to end (increased timeout for ASAN compatibility) + wait_for_condition $failover_timeout 100 { [s 0 master_failover_state] == "no-failover" } else { fail "Failover from node_0 to replica did not finish" diff --git a/tests/swap/support/util.tcl b/tests/swap/support/util.tcl index bc7d780986b..3a544fe4d18 100644 --- a/tests/swap/support/util.tcl +++ b/tests/swap/support/util.tcl @@ -477,7 +477,7 @@ proc scan_all_keys {r} { while {1} { set res [$r scan $cursor] set cursor [lindex $res 0] - lappend keys {*}[split [lindex $res 1] " "] + lappend keys {*}[lindex $res 1] if {$cursor == 0} { break } @@ -497,36 +497,36 @@ proc swap_data_comp {r1 r2} { assert_equal [$r1 dbsize] [$r2 dbsize] set keys [scan_all_keys $r1] foreach key $keys { - set t [$r1 type {*}$key] - set t2 [$r2 type {*}$key] + set t [$r1 type $key] + set t2 [$r2 type $key] if {$t != $t2} { assert_failed "key '$key' type mismatch '$t' - '$t2'" "" } switch $t { {string} { - set v1 [$r1 get {*}$key] - set v2 [$r2 get {*}$key] + set v1 [$r1 get $key] + set v2 [$r2 get $key] if {$v1 != $v2} { data_conflict $t $key '' $v1 $v2 } } {list} { - set len [$r1 llen {*}$key] - set len2 [$r2 llen {*}$key] + set len [$r1 llen $key] + set len2 [$r2 llen $key] if {$len != $len2} { data_conflict $t $key '' 'LLEN:$len' 'LLEN:$len2' } for {set i 0} {$i < $len} {incr i} { - set v1 [$r1 lindex {*}$key $i] - set v2 [$r2 lindex {*}$key $i] + set v1 [$r1 lindex $key $i] + set v2 [$r2 lindex $key $i] if {$v1 != $v2} { data_conflict $t $key $i $v1 $v2 } } } {set} { - set len [$r1 scard {*}$key] - set len2 [$r2 scard {*}$key] + set len [$r1 scard $key] + set len2 [$r2 scard $key] if {$len != $len2} { data_conflict $t $key '' 'SLEN:$len' 'SLEN:$len2' } @@ -538,21 +538,21 @@ proc swap_data_comp {r1 r2} { } } {zset} { - set len [$r1 zcard {*}$key] - set len2 [$r2 zcard {*}$key] + set len [$r1 zcard $key] + set len2 [$r2 zcard $key] if {$len != $len2} { data_conflict $t $key '' 'SLEN:$len' 'SLEN:$len2' } set zcursor 0 while 1 { - set res [$r1 zscan {*}$key $zcursor] + set res [$r1 zscan $key $zcursor] set zcursor [lindex $res 0] set zdata [lindex $res 1] set dlen [llength $zdata] for {set i 0} {$i < $dlen} {incr i 2} { set zkey [lindex $zdata $i] set zscore [lindex $zdata [expr $i+1]] - set zscore2 [$r2 zscore {*}$key $zkey] + set zscore2 [$r2 zscore $key $zkey] if {$zscore != $zscore2} { data_conflict $t $key $zkey $zscore $zscore2 } @@ -563,15 +563,15 @@ proc swap_data_comp {r1 r2} { } } {hash} { - set len [$r1 hlen {*}$key] - set len2 [$r2 hlen {*}$key] + set len [$r1 hlen $key] + set len2 [$r2 hlen $key] if {$len != $len2} { data_conflict $t $key '' 'HLEN:$len' 'HLEN:$len2' } - set hkeys [$r1 hkeys {*}$key] + set hkeys [$r1 hkeys $key] foreach hkey $hkeys { - set v1 [$r1 hget {*}$key $hkey] - set v2 [$r2 hget {*}$key $hkey] + set v1 [$r1 hget $key $hkey] + set v2 [$r2 hget $key $hkey] if {$v1 != $v2} { data_conflict $t $key $hkey $v1 $v2 } diff --git a/tests/swap/unit/del.tcl b/tests/swap/unit/del.tcl index 3f6f6d326a8..712c5bf6d87 100644 --- a/tests/swap/unit/del.tcl +++ b/tests/swap/unit/del.tcl @@ -70,7 +70,7 @@ start_server {tags {"skip_unnecessary_rocksdb_del"}} { while {1} { set res [$r scan $cursor] set cursor [lindex $res 0] - lappend keys {*}[split [lindex $res 1] " "] + lappend keys {*}[lindex $res 1] if {$cursor == 0} { break } diff --git a/tests/swap/unit/expire.tcl b/tests/swap/unit/expire.tcl index 9340bb48eaf..ebbf583f572 100644 --- a/tests/swap/unit/expire.tcl +++ b/tests/swap/unit/expire.tcl @@ -4,7 +4,7 @@ proc scan_all_keys {r} { while {1} { set res [$r scan $cursor] set cursor [lindex $res 0] - append keys [lindex $res 1] + lappend keys {*}[lindex $res 1] if {$cursor == 0} { break } diff --git a/tests/test_helper.tcl b/tests/test_helper.tcl index 883eab0f679..2e241a8bca8 100644 --- a/tests/test_helper.tcl +++ b/tests/test_helper.tcl @@ -78,6 +78,7 @@ set ::portcount 8000; # we don't wanna use more than 10000 to avoid collision wi set ::traceleaks 0 set ::valgrind 0 set ::tsan 0 +set ::asan 0 set ::durable 0 set ::tls 0 set ::tls_module 0 @@ -650,6 +651,8 @@ for {set j 0} {$j < [llength $argv]} {incr j} { set ::valgrind 1 } elseif {$opt eq {--tsan}} { set ::tsan 1 + } elseif {$opt eq {--asan}} { + set ::asan 1 } elseif {$opt eq {--stack-logging}} { if {[string match {*Darwin*} [exec uname -a]]} { set ::stack_logging 1