Correctly cloexec file redirections

The IO cleanup left file redirections open in the child. For example,
/bin/cmd < file.txt would redirect stdin but also leave the file open.
Ensure these get closed properly.

src/fish_test_helper.cpp

@@ -1,6 +1,7 @@
 // fish_test_helper is a little program with no fish dependencies that acts like certain other
 // programs, allowing fish to test its behavior.
 
+#include <fcntl.h>
 #include <unistd.h>
 
 #include <csignal>
@@ -54,6 +55,17 @@ static void print_pid_then_sleep() {
 
 static void print_pgrp() { fprintf(stdout, "%d\n", getpgrp()); }
 
+static void print_fds() {
+    bool needs_space = false;
+    for (int fd = 0; fd <= 100; fd++) {
+        if (fcntl(fd, F_GETFD) >= 0) {
+            fprintf(stdout, "%s%d", needs_space ? " " : "", fd);
+            needs_space = true;
+        }
+    }
+    fputc('\n', stdout);
+}
+
 int main(int argc, char *argv[]) {
     if (argc <= 1) {
         fprintf(stderr, "No commands given.\n");
@@ -72,6 +84,8 @@ int main(int argc, char *argv[]) {
             print_pid_then_sleep();
         } else if (!strcmp(argv[i], "print_pgrp")) {
             print_pgrp();
+        } else if (!strcmp(argv[i], "print_fds")) {
+            print_fds();
         } else {
             fprintf(stderr, "%s: Unknown command: %s\n", argv[0], argv[i]);
             return EXIT_FAILURE;

src/io.cpp

@@ -255,9 +255,10 @@ bool io_chain_t::append_from_specs(const redirection_spec_list_t &specs, const w
             }
             default: {
                 // We have a path-based redireciton. Resolve it to a file.
+                // Mark it as CLO_EXEC because we don't want it to be open in any child.
                 wcstring path = path_apply_working_directory(spec.target, pwd);
                 int oflags = spec.oflags();
-                autoclose_fd_t file{wopen(path, oflags, OPEN_MASK)};
+                autoclose_fd_t file{wopen_cloexec(path, oflags, OPEN_MASK)};
                 if (!file.valid()) {
                     if ((oflags & O_EXCL) && (errno == EEXIST)) {
                         debug(1, NOCLOB_ERROR, spec.target.c_str());

src/redirection.h

@@ -66,6 +66,9 @@ class dup2_list_t {
     /// Append a dup2 action.
     void add_dup2(int src, int target) {
         assert(src >= 0 && target >= 0 && "Invalid fd in add_dup2");
+        // TODO: this is sloppy about CLO_EXEC. For example if the user does this:
+        //   /bin/stuff 6< file.txt
+        // and file.txt happens to get fd 6, then the file will be closed.
         if (src != target) {
             actions_.push_back(action_t{src, target});
         }

tests/checks/fds.fish

@@ -0,0 +1,24 @@
+# RUN: %fish -C "set helper %fish_test_helper" %s
+
+# Check that we don't leave stray FDs.
+
+$helper print_fds
+# CHECK: 0 1 2
+
+$helper print_fds 0>&-
+# CHECK: 1 2
+
+$helper print_fds 0>&- 2>&-
+# CHECK: 1
+
+false | $helper print_fds 0>&-
+# CHECK: 0 1 2
+
+$helper print_fds <(status current-filename)
+# CHECK: 0 1 2
+
+$helper print_fds <(status current-filename)
+# CHECK: 0 1 2
+
+$helper print_fds 3<(status current-filename)
+# CHECK: 0 1 2 3