Merge pull request #30 from ksss/cloexec

Opened fd should be set FD_CLOEXEC by default

src/io.c

@@ -125,6 +125,30 @@ mrb_io_flags_to_modenum(mrb_state *mrb, int flags)
   return modenum;
 }
 
+void
+mrb_fd_cloexec(mrb_state *mrb, int fd)
+{
+  int flags, flags2;
+
+#if defined(F_GETFD) && defined(F_SETFD) && defined(FD_CLOEXEC)
+  flags = fcntl(fd, F_GETFD);
+  if (flags == -1) {
+    mrb_sys_fail(mrb, "fcntl");
+  }
+  if (fd <= 2) {
+    flags2 = flags & ~FD_CLOEXEC; /* Clear CLOEXEC for standard file descriptors: 0, 1, 2. */
+  }
+  else {
+    flags2 = flags | FD_CLOEXEC; /* Set CLOEXEC for non-standard file descriptors: 3, 4, 5, ... */
+  }
+  if (flags != flags2) {
+    if (fcntl(fd, F_SETFD, flags2) == -1) {
+      mrb_sys_fail(mrb, "fcntl");
+    }
+  }
+#endif
+}
+
 #ifndef _WIN32
 static int
 mrb_proc_exec(const char *pname)
@@ -199,13 +223,22 @@ mrb_io_s_popen(mrb_state *mrb, mrb_value klass)
 
   doexec = (strcmp("-", pname) != 0);
 
-  if ((flags & FMODE_READABLE) && pipe(pr) == -1) {
-    mrb_sys_fail(mrb, "pipe");
+  if (flags & FMODE_READABLE) {
+    if (pipe(pr) == -1) {
+      mrb_sys_fail(mrb, "pipe");
+    }
+    mrb_fd_cloexec(mrb, pr[0]);
+    mrb_fd_cloexec(mrb, pr[1]);
   }
-  if ((flags & FMODE_WRITABLE) && pipe(pw) == -1) {
-    if (pr[0] != -1) close(pr[0]);
-    if (pr[1] != -1) close(pr[1]);
-    mrb_sys_fail(mrb, "pipe");
+
+  if (flags & FMODE_WRITABLE) {
+    if (pipe(pw) == -1) {
+      if (pr[0] != -1) close(pr[0]);
+      if (pr[1] != -1) close(pr[1]);
+      mrb_sys_fail(mrb, "pipe");
+    }
+    mrb_fd_cloexec(mrb, pw[0]);
+    mrb_fd_cloexec(mrb, pw[1]);
   }
 
   if (!doexec) {
@@ -389,6 +422,38 @@ mrb_io_s_sysclose(mrb_state *mrb, mrb_value klass)
   return mrb_fixnum_value(0);
 }
 
+int
+mrb_cloexec_open(mrb_state *mrb, const char *pathname, mrb_int flags, mrb_int mode)
+{
+  int fd, retry = FALSE;
+
+#ifdef O_CLOEXEC
+  /* O_CLOEXEC is available since Linux 2.6.23.  Linux 2.6.18 silently ignore it. */
+  flags |= O_CLOEXEC;
+#elif defined O_NOINHERIT
+  flags |= O_NOINHERIT;
+#endif
+reopen:
+  fd = open(pathname, flags, mode);
+  if (fd == -1) {
+    if (!retry) {
+      switch (errno) {
+        case ENFILE:
+        case EMFILE:
+        mrb_garbage_collect(mrb);
+        retry = TRUE;
+        goto reopen;
+      }
+    }
+    mrb_sys_fail(mrb, "open");
+  }
+
+  if (fd <= 2) {
+    mrb_fd_cloexec(mrb, fd);
+  }
+  return fd;
+}
+
 mrb_value
 mrb_io_s_sysopen(mrb_state *mrb, mrb_value klass)
 {
@@ -396,7 +461,7 @@ mrb_io_s_sysopen(mrb_state *mrb, mrb_value klass)
   mrb_value mode = mrb_nil_value();
   mrb_int fd, flags, perm = -1;
   const char *pat;
-  int modenum, retry = FALSE;
+  int modenum;
 
   mrb_get_args(mrb, "S|Si", &path, &mode, &perm);
   if (mrb_nil_p(mode)) {
@@ -409,22 +474,7 @@ mrb_io_s_sysopen(mrb_state *mrb, mrb_value klass)
   pat = mrb_string_value_cstr(mrb, &path);
   flags = mrb_io_modestr_to_flags(mrb, mrb_string_value_cstr(mrb, &mode));
   modenum = mrb_io_flags_to_modenum(mrb, flags);
-
- reopen:
-  fd = open(pat, modenum, perm);
-  if (fd == -1) {
-    if (!retry) {
-      switch (errno) {
-      case ENFILE:
-      case EMFILE:
-        mrb_garbage_collect(mrb);
-        retry = TRUE;
-        goto reopen;
-      }
-    }
-    mrb_sys_fail(mrb, pat);
-  }
-
+  fd = mrb_cloexec_open(mrb, pat, modenum, perm);
   return mrb_fixnum_value(fd);
 }
 

test/io.rb

@@ -321,6 +321,7 @@ end
 
 assert('IO.popen') do
   io = IO.popen("ls")
+  assert_true io.close_on_exec?
   assert_equal Fixnum, io.pid.class
   ls = io.read
   assert_equal ls.class, String
@@ -362,28 +363,28 @@ assert('IO#fileno') do
   io.closed?
 end
 
-assert('IO#close_on_exec') do 
+assert('IO#close_on_exec') do
   fd = IO.sysopen $mrbtest_io_wfname, "w"
   io = IO.new fd, "w"
-  begin 
-    # IO.sysopen opens a file descripter without O_CLOEXEC flag.
-    assert_equal(false, io.close_on_exec?)
+  begin
+    # IO.sysopen opens a file descripter with O_CLOEXEC flag.
+    assert_true io.close_on_exec?
   rescue ScriptError
     skip "IO\#close_on_exec is not implemented."
   end
 
-  io.close_on_exec = true
-  assert_equal(true, io.close_on_exec?)
   io.close_on_exec = false
   assert_equal(false, io.close_on_exec?)
   io.close_on_exec = true
   assert_equal(true, io.close_on_exec?)
-  
+  io.close_on_exec = false
+  assert_equal(false, io.close_on_exec?)
+
   io.close
   io.closed?
 
   # # Use below when IO.pipe is implemented.
-  # begin 
+  # begin
   #   r, w = IO.pipe
   #   assert_equal(false, r.close_on_exec?)
   #   r.close_on_exec = true