Skip to content

gspawn: Implement fd remapping for posix_spawn codepath, and fix file descriptor conflation issues

Michael Catanzaro requested to merge mcatanzaro/posix-spawn2 into main

Hi Philip, this removes the FIXME that you added in !1690 (merged). I think the implementation is on par with the existing code, unless there is some edge case I've failed to consider.

But it's marked WIP because I discovered a (quite unlikely) edge case in the preexisting fork/exec codepath that I want to investigate further before landing:

  /* FIXME: this works in the naive case but not for a case like:
   *
   * Case: source_fd 5 -> target_fd 7, source_fd 4 -> target_fd 6
   *
   * dup(5) -> get unlucky, dup returns 6
   * dup(4) -> dup returns 7
   *
   * This means we wind up with: 6 -> 7, 7 -> 6. That means that after
   * we dup2(6, 7), we have clobbered fd 7 before we dup2(7, 6). End
   * result is we have remapped 5 -> 7 as expected, but then remapped
   * 5 -> 6 instead of 4 -> 6. So we will need a quadratic loop to check
   * that each value in duped_source_fds is not present in target_fds,
   * and repeatedly dup if it is.
   *
   * The existing fork/exec codepath has the exact same bug and must
   * also be fixed.
   */

I'll probably open a separate issue for that after thinking harder about it. Seemed useful to create the MR now as a heads-up since I see you've been working on the code recently.

Update: I wound up fixing the above as part of this MR.

Edited by Michael Catanzaro

Merge request reports