unix: don't close fd immediately after dup2() in uv_spawn() #923

Activity

1. indutny commented on Sep 7, 2013

   

   indutny

   on Sep 7, 2013

   Contributor

   More actions

   Mind if I'll take a look at it?
2. ghost assigned

   indutny

   on Sep 7, 2013
3. bnoordhuis commented on Sep 7, 2013

   

   bnoordhuis

   on Sep 7, 2013

   ContributorAuthor

   More actions

   Sure, go ahead.
4. bnoordhuis commented on Sep 7, 2013

   

   bnoordhuis

   on Sep 7, 2013

   ContributorAuthor

   More actions

   I'm reasonably sure the patch below fixes it but I'm unsure as to how correct it is. It passes all node and libuv tests but that doesn't mean it doesn't have bugs, of course.

   diff --git a/src/unix/process.c b/src/unix/process.c
   index 990ea15..4d86089 100644
   --- a/src/unix/process.c
   +++ b/src/unix/process.c
   @@ -285,8 +285,10 @@ static void uv__process_child_init(const uv_process_options_t* options,
        close_fd = pipes[fd][0];
        use_fd = pipes[fd][1];
   
   -    if (use_fd >= 0)
   -      close(close_fd);
   +    if (use_fd >= 0) {
   +      if (close_fd != -1)
   +        close(close_fd);
   +    }
        else if (fd >= 3)
          continue;
        else {
   @@ -304,15 +306,22 @@ static void uv__process_child_init(const uv_process_options_t* options,
   
        if (fd == use_fd)
          uv__cloexec(use_fd, 0);
   -    else {
   +    else
          dup2(use_fd, fd);
   -      close(use_fd);
   -    }
   
        if (fd <= 2)
          uv__nonblock(fd, 0);
      }
   
   +  for (fd = 0; fd < stdio_count; fd++) {
   +    if (fd >= 3)
   +      break;
   +
   +    use_fd = pipes[fd][1];
   +    if (fd != use_fd)
   +      close(use_fd);
   +  }
   +
      if (options->cwd != NULL && chdir(options->cwd)) {
        uv__write_int(error_fd, -errno);
        perror("chdir()");
5. indutny commented on Sep 10, 2013

   

   indutny

   on Sep 10, 2013

   Contributor

   More actions

   @bnoordhuis LGTM
6. bnoordhuis commented on Sep 10, 2013

   

   bnoordhuis

   on Sep 10, 2013

   ContributorAuthor

   More actions

   I found another bug in uv__process_child_init(), let me follow up with a more thorough patch.
7. ghost assigned

   bnoordhuis

   on Sep 10, 2013
8. bnoordhuis

   removed their assignment

   on May 12, 2015
9. bnoordhuis

   closed this as completedon May 12, 2015