ngoto	2015-07-15 23:26:22 +0900 (Wed, 15 Jul 2015)

  New Revision: 51255

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=51255

  Log:
    * process.c (redirect_close, parent_redirect_close): should not close
      reserved FD. It should be closed in the exec system call due to the
      O_CLOEXEC or FD_CLOEXEC flag. [Bug #11353] [ruby-core:69977]
        
    * process.c (close_unless_reserved): new function to close FD unless
      it is reserved for internal communication.
          
    * thread_pthread.c (rb_reserved_fd_p): should check owner_process pid
      to avoid false positive in forked child process.

  Modified files:
    trunk/ChangeLog
    trunk/process.c
    trunk/thread_pthread.c
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 51254)
+++ ChangeLog	(revision 51255)
@@ -1,3 +1,15 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Wed Jul 15 23:01:22 2015  Naohisa Goto  <ngotogenome@g...>
+
+	* process.c (redirect_close, parent_redirect_close): should not close
+	  reserved FD. It should be closed in the exec system call due to the
+	  O_CLOEXEC or FD_CLOEXEC flag. [Bug #11353] [ruby-core:69977]
+
+	* process.c (close_unless_reserved): new function to close FD unless
+	  it is reserved for internal communication.
+
+	* thread_pthread.c (rb_reserved_fd_p): should check owner_process pid
+	  to avoid false positive in forked child process.
+
 Wed Jul 15 18:31:18 2015  Eric Wong  <e@8...>
 
 	* proc.c (proc_mark): remove redundant check
Index: thread_pthread.c
===================================================================
--- thread_pthread.c	(revision 51254)
+++ thread_pthread.c	(revision 51255)
@@ -1692,10 +1692,11 @@ int https://github.com/ruby/ruby/blob/trunk/thread_pthread.c#L1692
 rb_reserved_fd_p(int fd)
 {
 #if USE_SLEEPY_TIMER_THREAD
-    if (fd == timer_thread_pipe.normal[0] ||
-	fd == timer_thread_pipe.normal[1] ||
-	fd == timer_thread_pipe.low[0] ||
-	fd == timer_thread_pipe.low[1]) {
+    if ((fd == timer_thread_pipe.normal[0] ||
+	 fd == timer_thread_pipe.normal[1] ||
+	 fd == timer_thread_pipe.low[0] ||
+	 fd == timer_thread_pipe.low[1]) &&
+	timer_thread_pipe.owner_process == getpid()) { /* async-signal-safe */
 	return 1;
     }
     else {
Index: process.c
===================================================================
--- process.c	(revision 51254)
+++ process.c	(revision 51255)
@@ -294,6 +294,16 @@ extern ID ruby_static_id_status; https://github.com/ruby/ruby/blob/trunk/process.c#L294
 #define ALWAYS_NEED_ENVP 0
 #endif
 
+static inline int close_unless_reserved(fd)
+{
+    /* Do nothing to the reserved fd because it should be closed in exec(2)
+       due to the O_CLOEXEC or FD_CLOEXEC flag. */
+    if (rb_reserved_fd_p(fd)) { /* async-signal-safe */
+        return 0;
+    }
+    return close(fd); /* async-signal-safe */
+}
+
 /*#define DEBUG_REDIRECT*/
 #if defined(DEBUG_REDIRECT)
 
@@ -342,7 +352,7 @@ static int https://github.com/ruby/ruby/blob/trunk/process.c#L352
 redirect_close(int fd)
 {
     int ret;
-    ret = close(fd);
+    ret = close_unless_reserved(fd);
     ttyprintf("close(%d) => %d\n", fd, ret);
     return ret;
 }
@@ -360,7 +370,7 @@ static int https://github.com/ruby/ruby/blob/trunk/process.c#L370
 parent_redirect_close(int fd)
 {
     int ret;
-    ret = close(fd);
+    ret = close_unless_reserved(fd);
     ttyprintf("parent_close(%d) => %d\n", fd, ret);
     return ret;
 }
@@ -368,9 +378,9 @@ parent_redirect_close(int fd) https://github.com/ruby/ruby/blob/trunk/process.c#L378
 #else
 #define redirect_dup(oldfd) dup(oldfd)
 #define redirect_dup2(oldfd, newfd) dup2((oldfd), (newfd))
-#define redirect_close(fd) close(fd)
+#define redirect_close(fd) close_unless_reserved(fd)
 #define parent_redirect_open(pathname, flags, perm) rb_cloexec_open((pathname), (flags), (perm))
-#define parent_redirect_close(fd) close(fd)
+#define parent_redirect_close(fd) close_unless_reserved(fd)
 #endif
 
 /*

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/