From 7de6451818175c41ed5cda5d54d7cb9ebb1a81ad Mon Sep 17 00:00:00 2001 From: Qu Chen Date: Wed, 24 Mar 2021 08:41:05 -0700 Subject: [PATCH] Properly initialize variable to make valgrind happy in checkChildrenDone(). Removed usage for the obsolete wait3() and wait4() in favor of waitpid(), and properly check for the exit status code. (#8666) --- src/aof.c | 2 +- src/module.c | 2 +- src/rdb.c | 2 +- src/sentinel.c | 4 ++-- src/server.c | 11 +++++------ tests/integration/replication.tcl | 2 +- 6 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/aof.c b/src/aof.c index 237c4034c..e15eca8e1 100644 --- a/src/aof.c +++ b/src/aof.c @@ -218,7 +218,7 @@ void killAppendOnlyChild(void) { serverLog(LL_NOTICE,"Killing running AOF rewrite child: %ld", (long) server.child_pid); if (kill(server.child_pid,SIGUSR1) != -1) { - while(wait3(&statloc,0,NULL) != server.child_pid); + while(waitpid(-1, &statloc, 0) != server.child_pid); } /* Reset the buffer accumulating changes while the child saves. */ aofRewriteBufferReset(); diff --git a/src/module.c b/src/module.c index 49b671885..4d6bdd0d4 100644 --- a/src/module.c +++ b/src/module.c @@ -7821,7 +7821,7 @@ int TerminateModuleForkChild(int child_pid, int wait) { serverLog(LL_VERBOSE,"Killing running module fork child: %ld", (long) server.child_pid); if (kill(server.child_pid,SIGUSR1) != -1 && wait) { - while(wait4(server.child_pid,&statloc,0,NULL) != + while(waitpid(server.child_pid, &statloc, 0) != server.child_pid); } /* Reset the buffer accumulating changes while the child saves. */ diff --git a/src/rdb.c b/src/rdb.c index 94f568bb3..bd5b8b21e 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -2732,7 +2732,7 @@ void backgroundSaveDoneHandler(int exitcode, int bysignal) { * the cleanup needed. */ void killRDBChild(void) { kill(server.child_pid, SIGUSR1); - /* Because we are not using here wait4 (like we have in killAppendOnlyChild + /* Because we are not using here waitpid (like we have in killAppendOnlyChild * and TerminateModuleForkChild), all the cleanup operations is done by * checkChildrenDone, that later will find that the process killed. * This includes: diff --git a/src/sentinel.c b/src/sentinel.c index 419b224d2..a23b5b328 100644 --- a/src/sentinel.c +++ b/src/sentinel.c @@ -904,7 +904,7 @@ void sentinelCollectTerminatedScripts(void) { int statloc; pid_t pid; - while ((pid = wait3(&statloc,WNOHANG,NULL)) > 0) { + while ((pid = waitpid(-1, &statloc, WNOHANG)) > 0) { int exitcode = WEXITSTATUS(statloc); int bysignal = 0; listNode *ln; @@ -916,7 +916,7 @@ void sentinelCollectTerminatedScripts(void) { ln = sentinelGetScriptListNodeByPid(pid); if (ln == NULL) { - serverLog(LL_WARNING,"wait3() returned a pid (%ld) we can't find in our scripts execution queue!", (long)pid); + serverLog(LL_WARNING,"waitpid() returned a pid (%ld) we can't find in our scripts execution queue!", (long)pid); continue; } sj = ln->value; diff --git a/src/server.c b/src/server.c index 35c012c12..21519a18d 100644 --- a/src/server.c +++ b/src/server.c @@ -1928,11 +1928,11 @@ void updateCachedTime(int update_daylight_info) { } void checkChildrenDone(void) { - int statloc; + int statloc = 0; pid_t pid; - if ((pid = wait3(&statloc,WNOHANG,NULL)) != 0) { - int exitcode = WEXITSTATUS(statloc); + if ((pid = waitpid(-1, &statloc, WNOHANG)) != 0) { + int exitcode = WIFEXITED(statloc) ? WEXITSTATUS(statloc) : -1; int bysignal = 0; if (WIFSIGNALED(statloc)) bysignal = WTERMSIG(statloc); @@ -1940,15 +1940,14 @@ void checkChildrenDone(void) { /* sigKillChildHandler catches the signal and calls exit(), but we * must make sure not to flag lastbgsave_status, etc incorrectly. * We could directly terminate the child process via SIGUSR1 - * without handling it, but in this case Valgrind will log an - * annoying error. */ + * without handling it */ if (exitcode == SERVER_CHILD_NOERROR_RETVAL) { bysignal = SIGUSR1; exitcode = 1; } if (pid == -1) { - serverLog(LL_WARNING,"wait3() returned an error: %s. " + serverLog(LL_WARNING,"waitpid() returned an error: %s. " "child_type: %s, child_pid = %d", strerror(errno), strChildType(server.child_type), diff --git a/tests/integration/replication.tcl b/tests/integration/replication.tcl index 62b175209..47d83f24a 100644 --- a/tests/integration/replication.tcl +++ b/tests/integration/replication.tcl @@ -722,7 +722,7 @@ start_server {tags {"repl"}} { test "diskless replication child being killed is collected" { # when diskless master is waiting for the replica to become writable # it removes the read event from the rdb pipe so if the child gets killed - # the replica will hung. and the master may not collect the pid with wait3 + # the replica will hung. and the master may not collect the pid with waitpid start_server {tags {"repl"}} { set master [srv 0 client] set master_host [srv 0 host]