title: wait3 (waitpid alias) returns -1 with errno set to ECHILD when it should not
body: <p>Context is this <a href="https://github.com/antirez/redis/issues/2897" rel="noreferrer">Redis issue</a>. We have a <code>wait3()</code> call that waits for the AOF rewriting child to create the new AOF version on disk. When the child is done, the parent is notified via <code>wait3()</code> in order to substitute the old AOF with the new one.</p>
  <p>However in the context of the above issue the user notified us about a bug. I modified a bit the implementation of Redis 3.0 in order to clearly log when <code>wait3()</code> returned -1 instead of crashing because of this unexpected condition. So this is what happens apparently:</p>
  <ol>
  <li><code>wait3()</code> is called when we have pending children to wait for.</li>
  <li>the <code>SIGCHLD</code> should be set to <code>SIG_DFL</code>, there is no code setting this signal at all in Redis, so it's the default behavior.</li>
  <li>When the first AOF rewrite happens, <code>wait3()</code> successfully works as expected.</li>
  <li>Starting from the second AOF rewrite (the second child created), <code>wait3()</code> starts to return -1.</li>
  </ol>
  <p>AFAIK it is not possible in the current code that we call <code>wait3()</code> while there are no pending children, since when the AOF child is created, we set <code>server.aof_child_pid</code> to the value of the pid, and we reset it only after a successful <code>wait3()</code> call.</p>
  <p>So <code>wait3()</code> should have no reason to fail with -1 and <code>ECHILD</code>, but it does, so probably the zombie child is not created for some unexpected reason.</p>
  <p><strong>Hypothesis 1</strong>: It is possible that Linux during certain odd conditions will discard the zombie child, for example because of memory pressure? Does not look reasonable since the zombie has just metadata attached to it but who knows.</p>
  <p>Note that we call <code>wait3()</code> with <code>WNOHANG</code>. And given that <code>SIGCHLD</code> is set to <code>SIG_DFL</code> by default, the only condition that should lead to failing and returning -1 and <code>ECHLD</code> should be no zombie available to report the information.</p>
  <p><strong>Hypothesis 2</strong>: Other thing that could happen but there is no explanation if it happens, is that after the first child dies, the <code>SIGCHLD</code> handler is set to <code>SIG_IGN</code>, causing <code>wait3()</code> to return -1 and <code>ECHLD</code>.</p>
  <p><strong>Hypothesis 3</strong>: Is there some way to remove the zombie children externally? Maybe this user has some kind of script that removes zombie processes in the background so that then the information is no longer available for <code>wait3()</code>? To my knowledge it should <em>never be possible</em> to remove the zombie if the parent does not wait for it (with <code>waitpid</code> or handling the signal) and if the <code>SIGCHLD</code> is not ignored, but maybe there is some Linux specific way.</p>
  <p><strong>Hypothesis 4</strong>: There is actually some bug in the Redis code so that we successfully <code>wait3()</code> the child the first time without correctly resetting the state, and later we call <code>wait3()</code> again and again but there are no longer zombies, so it returns -1. Analyzing the code it looks impossible, but maybe I'm wrong.</p>
  <p>Another important thing: <em>we never observed this in the past</em>. Only happens in this specific Linux system apparently.</p>
  <p><strong>UPDATE</strong>: Yossi Gottlieb proposed that the <code>SIGCHLD</code> is received by another thread in the Redis process for some reason (does not happen normally, only on this system). We already mask <code>SIGALRM</code> in <code>bio.c</code> threads, perhaps we could try masking <code>SIGCHLD</code> from I/O threads as well.</p>
  <h1>Appendix: selected parts of Redis code</h1>
  <p>Where wait3() is called:</p>
  <pre><code>/* Check if a background saving or AOF rewrite in progress terminated. */
  if (server.rdb_child_pid != -1 || server.aof_child_pid != -1) {
      int statloc;
      pid_t pid;
  
      if ((pid = wait3(&amp;statloc,WNOHANG,NULL)) != 0) {
          int exitcode = WEXITSTATUS(statloc);
          int bysignal = 0;
  
          if (WIFSIGNALED(statloc)) bysignal = WTERMSIG(statloc);
  
          if (pid == -1) {
              redisLog(LOG_WARNING,&quot;wait3() returned an error: %s. &quot;
                  &quot;rdb_child_pid = %d, aof_child_pid = %d&quot;,
                  strerror(errno),
                  (int) server.rdb_child_pid,
                  (int) server.aof_child_pid);
          } else if (pid == server.rdb_child_pid) {
              backgroundSaveDoneHandler(exitcode,bysignal);
          } else if (pid == server.aof_child_pid) {
              backgroundRewriteDoneHandler(exitcode,bysignal);
          } else {
              redisLog(REDIS_WARNING,
                  &quot;Warning, detected child with unmatched pid: %ld&quot;,
                  (long)pid);
          }
          updateDictResizePolicy();
      }
  } else {
  </code></pre>
  <p>Selected parts of <code>backgroundRewriteDoneHandler</code>:</p>
  <pre><code>void backgroundRewriteDoneHandler(int exitcode, int bysignal) {
      if (!bysignal &amp;&amp; exitcode == 0) {
          int newfd, oldfd;
          char tmpfile[256];
          long long now = ustime();
          mstime_t latency;
  
          redisLog(REDIS_NOTICE,
              &quot;Background AOF rewrite terminated with success&quot;);
  
          ... more code to handle the rewrite, never calls return ...
  
      } else if (!bysignal &amp;&amp; exitcode != 0) {
          server.aof_lastbgrewrite_status = REDIS_ERR;
  
          redisLog(REDIS_WARNING,
              &quot;Background AOF rewrite terminated with error&quot;);
      } else {
          server.aof_lastbgrewrite_status = REDIS_ERR;
  
          redisLog(REDIS_WARNING,
              &quot;Background AOF rewrite terminated by signal %d&quot;, bysignal);
      }
  
  cleanup:
      aofClosePipes();
      aofRewriteBufferReset();
      aofRemoveTempFile(server.aof_child_pid);
      server.aof_child_pid = -1;
      server.aof_rewrite_time_last = time(NULL)-server.aof_rewrite_time_start;
      server.aof_rewrite_time_start = -1;
      /* Schedule a new rewrite if we are waiting for it to switch the AOF ON. */
      if (server.aof_state == REDIS_AOF_WAIT_REWRITE)
          server.aof_rewrite_scheduled = 1;
  }
  </code></pre>
  <p>As you can see all the code paths must execute the <code>cleanup</code> code that reset <code>server.aof_child_pid</code> to -1.</p>
  <h1>Errors logged by Redis during the issue</h1>
  <blockquote>
  <p>21353:C 29 Nov 04:00:29.957 * AOF rewrite: 8 MB of memory used by copy-on-write</p>
  <p>27848:M 29 Nov 04:00:30.133 ^@ wait3() returned an error: No child processes. rdb_child_pid = -1, aof_child_pid = 21353</p>
  </blockquote>
  <p>As you can see <code>aof_child_pid</code> is not -1.</p>