From 9f3c422c72d4cede717b5f61bb7bde5823db53f2 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Fri, 5 Jun 2009 22:52:30 +0200
Subject: [PATCH] Kill the background saving process before performing SHUTDOWN
 to avoid races

---
 benchmark.c          | 18 +++++++++---------
 doc/LremCommand.html |  2 +-
 redis.c              | 20 +++++++++++++++-----
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/benchmark.c b/benchmark.c
index 7d4c844d..0d53c935 100644
--- a/benchmark.c
+++ b/benchmark.c
@@ -425,15 +425,6 @@ int main(int argc, char **argv) {
     }
 
     do {
-        prepareForBenchmark();
-        c = createClient();
-        if (!c) exit(1);
-        c->obuf = sdscat(c->obuf,"PING\r\n");
-        c->replytype = REPLY_RETCODE;
-        createMissingClients(c);
-        aeMain(config.el);
-        endBenchmark("PING");
-
         prepareForBenchmark();
         c = createClient();
         if (!c) exit(1);
@@ -488,6 +479,15 @@ int main(int argc, char **argv) {
         aeMain(config.el);
         endBenchmark("LPOP");
 
+        prepareForBenchmark();
+        c = createClient();
+        if (!c) exit(1);
+        c->obuf = sdscat(c->obuf,"PING\r\n");
+        c->replytype = REPLY_RETCODE;
+        createMissingClients(c);
+        aeMain(config.el);
+        endBenchmark("PING");
+
         printf("\n");
     } while(config.loop);
 
diff --git a/doc/LremCommand.html b/doc/LremCommand.html
index f0585ef7..c7d6c878 100644
--- a/doc/LremCommand.html
+++ b/doc/LremCommand.html
@@ -27,7 +27,7 @@
 
                 <div class="narrow">
                     <h1><a name="LREM _key_ _count_ _value_">LREM _key_ _count_ _value_</a></h1>
-<i>Time complexity: O(N) (with N being the length of the list)</i><blockquote>Remove the first <i>count</i> occurrences of the <i>value</i> element from the list.If <i>count</i> is zero all the elements are removed. If <i>count</i> is negativeelements are removed from tail to head, instead to go from head to tailthat is the normal behaviour. So for example LREM with count -2 and_hello_ as value to remove against the list (a,b,c,hello,x,hello,hello) willlave the list (a,b,c,hello,x). The number of removed elements is returnedas an integer, see below for more information aboht the returned value.</blockquote>
+<i>Time complexity: O(N) (with N being the length of the list)</i><blockquote>Remove the first <i>count</i> occurrences of the <i>value</i> element from the list.If <i>count</i> is zero all the elements are removed. If <i>count</i> is negativeelements are removed from tail to head, instead to go from head to tailthat is the normal behaviour. So for example LREM with count -2 and_hello_ as value to remove against the list (a,b,c,hello,x,hello,hello) willlave the list (a,b,c,hello,x). The number of removed elements is returnedas an integer, see below for more information about the returned value.Note that non existing keys are considered like empty lists by LREM, so LREMagainst non existing keys will always return 0.</blockquote>
 <h2><a name="Return value">Return value</a></h2><a href="ReplyTypes.html">Integer Reply</a>, specifically:<br/><br/><pre class="codeblock python" name="code">
 The number of removed elements if the operation succeeded
 </pre><h2><a name="See also">See also</a></h2>
diff --git a/redis.c b/redis.c
index 8b3f3be2..ff8db0be 100644
--- a/redis.c
+++ b/redis.c
@@ -240,6 +240,7 @@ struct redisServer {
     int daemonize;
     char *pidfile;
     int bgsaveinprogress;
+    pid_t bgsavechildpid;
     struct saveparam *saveparams;
     int saveparamslen;
     char *logfile;
@@ -748,16 +749,21 @@ int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) {
         /* XXX: TODO handle the case of the saving child killed */
         if (wait4(-1,&statloc,WNOHANG,NULL)) {
             int exitcode = WEXITSTATUS(statloc);
-            if (exitcode == 0) {
+            int bysignal = WIFSIGNALED(statloc);
+
+            if (!bysignal && exitcode == 0) {
                 redisLog(REDIS_NOTICE,
                     "Background saving terminated with success");
                 server.dirty = 0;
                 server.lastsave = time(NULL);
+            } else if (!bysignal && exitcode != 0) {
+                redisLog(REDIS_WARNING, "Background saving error");
             } else {
                 redisLog(REDIS_WARNING,
-                    "Background saving error");
+                    "Background saving terminated by signal");
             }
             server.bgsaveinprogress = 0;
+            server.bgsavechildpid = -1;
             updateSalvesWaitingBgsave(exitcode == 0 ? REDIS_OK : REDIS_ERR);
         }
     } else {
@@ -919,6 +925,7 @@ static void initServer() {
     }
     server.cronloops = 0;
     server.bgsaveinprogress = 0;
+    server.bgsavechildpid = -1;
     server.lastsave = time(NULL);
     server.dirty = 0;
     server.usedmemory = 0;
@@ -1983,6 +1990,7 @@ static int rdbSaveBackground(char *filename) {
         }
         redisLog(REDIS_NOTICE,"Background saving started by pid %d",childpid);
         server.bgsaveinprogress = 1;
+        server.bgsavechildpid = childpid;
         return REDIS_OK;
     }
     return REDIS_OK; /* unreached */
@@ -2505,11 +2513,13 @@ static void bgsaveCommand(redisClient *c) {
 
 static void shutdownCommand(redisClient *c) {
     redisLog(REDIS_WARNING,"User requested shutdown, saving DB...");
-    /* XXX: TODO kill the child if there is a bgsave in progress */
+    if (server.bgsaveinprogress) {
+        redisLog(REDIS_WARNING,"There is a live saving child. Killing it!");
+        kill(server.bgsavechildpid,SIGKILL);
+    }
     if (rdbSave(server.dbfilename) == REDIS_OK) {
-        if (server.daemonize) {
+        if (server.daemonize)
             unlink(server.pidfile);
-        }
         redisLog(REDIS_WARNING,"%zu bytes used at exit",zmalloc_used_memory());
         redisLog(REDIS_WARNING,"Server exit now, bye bye...");
         exit(1);
-- 
2.45.2