From a8b6596d233556980adc09589c9022e8a7901755 Mon Sep 17 00:00:00 2001 From: Andy Pan Date: Mon, 26 Apr 2021 20:52:06 +0800 Subject: [PATCH] Fail fast when systemic error occurs in poll (#8749) Most of the ae.c backends didn't explicitly handle errors, and instead ignored all errors and did an implicit retry. This is desired for EAGAIN and EINTER, but in case of other systematic errors, we prefer to fail and log the error we got rather than get into a busy loop. --- src/Makefile | 4 ++-- src/ae.c | 1 + src/ae_epoll.c | 3 +++ src/ae_evport.c | 3 +-- src/ae_kqueue.c | 3 +++ src/ae_select.c | 3 +++ src/redis-benchmark.c | 7 ------ src/redis-cli.c | 7 ------ src/redisassert.c | 52 +++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 src/redisassert.c diff --git a/src/Makefile b/src/Makefile index 28d50da02..ab9b834ad 100644 --- a/src/Makefile +++ b/src/Makefile @@ -288,9 +288,9 @@ REDIS_SERVER_NAME=redis-server$(PROG_SUFFIX) REDIS_SENTINEL_NAME=redis-sentinel$(PROG_SUFFIX) REDIS_SERVER_OBJ=adlist.o quicklist.o ae.o anet.o dict.o server.o sds.o zmalloc.o lzf_c.o lzf_d.o pqsort.o zipmap.o sha1.o ziplist.o release.o networking.o util.o object.o db.o replication.o rdb.o t_string.o t_list.o t_set.o t_zset.o t_hash.o config.o aof.o pubsub.o multi.o debug.o sort.o intset.o syncio.o cluster.o crc16.o endianconv.o slowlog.o scripting.o bio.o rio.o rand.o memtest.o crcspeed.o crc64.o bitops.o sentinel.o notify.o setproctitle.o blocked.o hyperloglog.o latency.o sparkline.o redis-check-rdb.o redis-check-aof.o geo.o lazyfree.o module.o evict.o expire.o geohash.o geohash_helper.o childinfo.o defrag.o siphash.o rax.o t_stream.o listpack.o localtime.o lolwut.o lolwut5.o lolwut6.o acl.o gopher.o tracking.o connection.o tls.o sha256.o timeout.o setcpuaffinity.o monotonic.o mt19937-64.o REDIS_CLI_NAME=redis-cli$(PROG_SUFFIX) -REDIS_CLI_OBJ=anet.o adlist.o dict.o redis-cli.o zmalloc.o release.o ae.o crcspeed.o crc64.o siphash.o crc16.o monotonic.o cli_common.o mt19937-64.o +REDIS_CLI_OBJ=anet.o adlist.o dict.o redis-cli.o zmalloc.o release.o ae.o redisassert.o crcspeed.o crc64.o siphash.o crc16.o monotonic.o cli_common.o mt19937-64.o REDIS_BENCHMARK_NAME=redis-benchmark$(PROG_SUFFIX) -REDIS_BENCHMARK_OBJ=ae.o anet.o redis-benchmark.o adlist.o dict.o zmalloc.o release.o crcspeed.o crc64.o siphash.o crc16.o monotonic.o cli_common.o mt19937-64.o +REDIS_BENCHMARK_OBJ=ae.o anet.o redis-benchmark.o adlist.o dict.o zmalloc.o redisassert.o release.o crcspeed.o crc64.o siphash.o crc16.o monotonic.o cli_common.o mt19937-64.o REDIS_CHECK_RDB_NAME=redis-check-rdb$(PROG_SUFFIX) REDIS_CHECK_AOF_NAME=redis-check-aof$(PROG_SUFFIX) diff --git a/src/ae.c b/src/ae.c index 48fb63ed9..4544ff959 100644 --- a/src/ae.c +++ b/src/ae.c @@ -32,6 +32,7 @@ #include "ae.h" #include "anet.h" +#include "redisassert.h" #include #include diff --git a/src/ae_epoll.c b/src/ae_epoll.c index 023c93a17..493ffcad2 100644 --- a/src/ae_epoll.c +++ b/src/ae_epoll.c @@ -127,7 +127,10 @@ static int aeApiPoll(aeEventLoop *eventLoop, struct timeval *tvp) { eventLoop->fired[j].fd = e->data.fd; eventLoop->fired[j].mask = mask; } + } else if (retval == -1 && errno != EINTR) { + panic("aeApiPoll: epoll_wait, %s", strerror(errno)); } + return numevents; } diff --git a/src/ae_evport.c b/src/ae_evport.c index 7a0b03aea..025409331 100644 --- a/src/ae_evport.c +++ b/src/ae_evport.c @@ -291,8 +291,7 @@ static int aeApiPoll(aeEventLoop *eventLoop, struct timeval *tvp) { return 0; /* Any other error indicates a bug. */ - perror("aeApiPoll: port_get"); - abort(); + panic("aeApiPoll: port_get, %s", strerror(errno)); } state->npending = nevents; diff --git a/src/ae_kqueue.c b/src/ae_kqueue.c index b146f2519..8666b03cf 100644 --- a/src/ae_kqueue.c +++ b/src/ae_kqueue.c @@ -130,7 +130,10 @@ static int aeApiPoll(aeEventLoop *eventLoop, struct timeval *tvp) { eventLoop->fired[j].fd = e->ident; eventLoop->fired[j].mask = mask; } + } else if (retval == -1 && errno != EINTR) { + panic("aeApiPoll: kevent, %s", strerror(errno)); } + return numevents; } diff --git a/src/ae_select.c b/src/ae_select.c index c039a8ea3..3cc746ec3 100644 --- a/src/ae_select.c +++ b/src/ae_select.c @@ -97,7 +97,10 @@ static int aeApiPoll(aeEventLoop *eventLoop, struct timeval *tvp) { eventLoop->fired[numevents].mask = mask; numevents++; } + } else if (retval == -1 && errno != EINTR) { + panic("aeApiPoll: select, %s", strerror(errno)); } + return numevents; } diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 068c094e1..9aff2966a 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -263,13 +263,6 @@ static int dictSdsKeyCompare(void *privdata, const void *key1, return memcmp(key1, key2, l1) == 0; } -/* _serverAssert is needed by dict */ -void _serverAssert(const char *estr, const char *file, int line) { - fprintf(stderr, "=== ASSERTION FAILED ==="); - fprintf(stderr, "==> %s:%d '%s' is not true",file,line,estr); - *((char*)-1) = 'x'; -} - static redisContext *getRedisContext(const char *ip, int port, const char *hostsocket) { diff --git a/src/redis-cli.c b/src/redis-cli.c index 5e12bd079..3f51c7139 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -476,13 +476,6 @@ void dictListDestructor(void *privdata, void *val) listRelease((list*)val); } -/* _serverAssert is needed by dict */ -void _serverAssert(const char *estr, const char *file, int line) { - fprintf(stderr, "=== ASSERTION FAILED ==="); - fprintf(stderr, "==> %s:%d '%s' is not true",file,line,estr); - *((char*)-1) = 'x'; -} - /*------------------------------------------------------------------------------ * Help functions *--------------------------------------------------------------------------- */ diff --git a/src/redisassert.c b/src/redisassert.c new file mode 100644 index 000000000..0e2da18ca --- /dev/null +++ b/src/redisassert.c @@ -0,0 +1,52 @@ +/* redisassert.c -- Implement the default _serverAssert and _serverPanic which + * simply print stack trace to standard error stream. + * + * This file is shared by those modules that try to print some logs about stack trace + * but don't have their own implementations of functions in redisassert.h. + * + * ---------------------------------------------------------------------------- + * + * Copyright (c) 2021, Andy Pan and Redis Labs + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Redis nor the names of its contributors may be used + * to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + + +#include +#include + +void _serverAssert(const char *estr, const char *file, int line) { + fprintf(stderr, "=== ASSERTION FAILED ==="); + fprintf(stderr, "==> %s:%d '%s' is not true",file,line,estr); + *((char*)-1) = 'x'; +} + +void _serverPanic(const char *file, int line, const char *msg, ...) { + fprintf(stderr, "------------------------------------------------"); + fprintf(stderr, "!!! Software Failure. Press left mouse button to continue"); + fprintf(stderr, "Guru Meditation: %s #%s:%d",msg,file,line); + abort(); +}