From deee2c1ef2249120ae7db0e7523bfad4041b21a6 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 3 May 2020 09:31:50 +0300 Subject: [PATCH] add daily github actions with libc malloc and valgrind * fix memlry leaks with diskless replica short read. * fix a few timing issues with valgrind runs * fix issue with valgrind and watchdog schedule signal about the valgrind WD issue: the stack trace test in logging.tcl, has issues with valgrind: ==28808== Can't extend stack to 0x1ffeffdb38 during signal delivery for thread 1: ==28808== too small or bad protection modes it seems to be some valgrind bug with SA_ONSTACK. SA_ONSTACK seems unneeded since WD is not recursive (SA_NODEFER was removed), also, not sure if it's even valid without a call to sigaltstack() --- .github/workflows/daily.yml | 48 +++++++++++++++++++++++ src/debug.c | 2 +- src/rdb.c | 60 +++++++++++++++++++++-------- tests/integration/aof.tcl | 12 ++++++ tests/integration/replication-3.tcl | 2 +- 5 files changed, 106 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/daily.yml diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml new file mode 100644 index 000000000..b6a9abb68 --- /dev/null +++ b/.github/workflows/daily.yml @@ -0,0 +1,48 @@ +name: Daily + +on: + schedule: + - cron: '0 7 * * *' + +jobs: + test-jemalloc: + runs-on: ubuntu-latest + timeout-minutes: 1200 + steps: + - uses: actions/checkout@v1 + - name: make + run: make + - name: test + run: | + sudo apt-get install tcl8.5 + ./runtest --accurate --verbose + - name: module api test + run: ./runtest-moduleapi --verbose + + test-libc-malloc: + runs-on: ubuntu-latest + timeout-minutes: 1200 + steps: + - uses: actions/checkout@v1 + - name: make + run: make MALLOC=libc + - name: test + run: | + sudo apt-get install tcl8.5 + ./runtest --accurate --verbose + - name: module api test + run: ./runtest-moduleapi --verbose + + test-valgrind: + runs-on: ubuntu-latest + timeout-minutes: 14400 + steps: + - uses: actions/checkout@v1 + - name: make + run: make valgrind + - name: test + run: | + sudo apt-get install tcl8.5 valgrind -y + ./runtest --valgrind --verbose --clients 1 + - name: module api test + run: ./runtest-moduleapi --valgrind --verbose --clients 1 diff --git a/src/debug.c b/src/debug.c index cbb56cb71..587ff7c5d 100644 --- a/src/debug.c +++ b/src/debug.c @@ -1636,7 +1636,7 @@ void enableWatchdog(int period) { /* Watchdog was actually disabled, so we have to setup the signal * handler. */ sigemptyset(&act.sa_mask); - act.sa_flags = SA_ONSTACK | SA_SIGINFO; + act.sa_flags = SA_SIGINFO; act.sa_sigaction = watchdogSignalHandler; sigaction(SIGALRM, &act, NULL); } diff --git a/src/rdb.c b/src/rdb.c index 9f6bf13f1..14c5579e0 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -1441,7 +1441,10 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { /* Load every single element of the list */ while(len--) { - if ((ele = rdbLoadEncodedStringObject(rdb)) == NULL) return NULL; + if ((ele = rdbLoadEncodedStringObject(rdb)) == NULL) { + decrRefCount(o); + return NULL; + } dec = getDecodedObject(ele); size_t len = sdslen(dec->ptr); quicklistPushTail(o->ptr, dec->ptr, len); @@ -1468,8 +1471,10 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { long long llval; sds sdsele; - if ((sdsele = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + if ((sdsele = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL) { + decrRefCount(o); + return NULL; + } if (o->encoding == OBJ_ENCODING_INTSET) { /* Fetch integer value from element. */ @@ -1508,13 +1513,23 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { double score; zskiplistNode *znode; - if ((sdsele = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + if ((sdsele = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL) { + decrRefCount(o); + return NULL; + } if (rdbtype == RDB_TYPE_ZSET_2) { - if (rdbLoadBinaryDoubleValue(rdb,&score) == -1) return NULL; + if (rdbLoadBinaryDoubleValue(rdb,&score) == -1) { + decrRefCount(o); + sdsfree(sdsele); + return NULL; + } } else { - if (rdbLoadDoubleValue(rdb,&score) == -1) return NULL; + if (rdbLoadDoubleValue(rdb,&score) == -1) { + decrRefCount(o); + sdsfree(sdsele); + return NULL; + } } /* Don't care about integer-encoded strings. */ @@ -1546,10 +1561,15 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { while (o->encoding == OBJ_ENCODING_ZIPLIST && len > 0) { len--; /* Load raw strings */ - if ((field = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; - if ((value = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + if ((field = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL) { + decrRefCount(o); + return NULL; + } + if ((value = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL) { + sdsfree(field); + decrRefCount(o); + return NULL; + } /* Add pair to ziplist */ o->ptr = ziplistPush(o->ptr, (unsigned char*)field, @@ -1577,10 +1597,15 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { while (o->encoding == OBJ_ENCODING_HT && len > 0) { len--; /* Load encoded strings */ - if ((field = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; - if ((value = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) - == NULL) return NULL; + if ((field = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL) { + decrRefCount(o); + return NULL; + } + if ((value = rdbGenericLoadStringObject(rdb,RDB_LOAD_SDS,NULL)) == NULL) { + sdsfree(field); + decrRefCount(o); + return NULL; + } /* Add pair to hash table */ ret = dictAdd((dict*)o->ptr, field, value); @@ -1600,7 +1625,10 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key) { while (len--) { unsigned char *zl = rdbGenericLoadStringObject(rdb,RDB_LOAD_PLAIN,NULL); - if (zl == NULL) return NULL; + if (zl == NULL) { + decrRefCount(o); + return NULL; + } quicklistAppendZiplist(o->ptr, zl); } } else if (rdbtype == RDB_TYPE_HASH_ZIPMAP || diff --git a/tests/integration/aof.tcl b/tests/integration/aof.tcl index 2734de7f1..b82c87d71 100644 --- a/tests/integration/aof.tcl +++ b/tests/integration/aof.tcl @@ -54,6 +54,12 @@ tags {"aof"} { set client [redis [dict get $srv host] [dict get $srv port] 0 $::tls] + wait_for_condition 50 100 { + [catch {$client ping} e] == 0 + } else { + fail "Loading DB is taking too much time." + } + test "Truncated AOF loaded: we expect foo to be equal to 5" { assert {[$client get foo] eq "5"} } @@ -71,6 +77,12 @@ tags {"aof"} { set client [redis [dict get $srv host] [dict get $srv port] 0 $::tls] + wait_for_condition 50 100 { + [catch {$client ping} e] == 0 + } else { + fail "Loading DB is taking too much time." + } + test "Truncated AOF loaded: we expect foo to be equal to 6 now" { assert {[$client get foo] eq "6"} } diff --git a/tests/integration/replication-3.tcl b/tests/integration/replication-3.tcl index 198e698f2..43eb53538 100644 --- a/tests/integration/replication-3.tcl +++ b/tests/integration/replication-3.tcl @@ -118,7 +118,7 @@ start_server {tags {"repl"}} { # correctly the RDB file: such file will contain "lua" AUX # sections with scripts already in the memory of the master. - wait_for_condition 50 100 { + wait_for_condition 500 100 { [s -1 master_link_status] eq {up} } else { fail "Replication not started."