kosaki	2011-05-28 22:52:03 +0900 (Sat, 28 May 2011)

  New Revision: 31761

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

  Log:
    * process.c (before_exec, after_exec): change SIGPIPE handler to SIG_DFL
      before calling execve(). Because r31760 reintroduced an issue that
      system() may hang up (i.e. [ruby-dev:12261]).
    * process.c (save_sigpipe, restore_sigpipe): new.

  Modified files:
    trunk/ChangeLog
    trunk/process.c
    trunk/test/ruby/test_process.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 31760)
+++ ChangeLog	(revision 31761)
@@ -1,3 +1,10 @@
+Sat May 28 19:30:17 2011  KOSAKI Motohiro  <kosaki.motohiro@g...>
+
+	* process.c (before_exec, after_exec): change SIGPIPE handler to SIG_DFL
+	  before calling execve(). Because r31760 reintroduced an issue that
+	  system() may hang up (i.e. [ruby-dev:12261]).
+	* process.c (save_sigpipe, restore_sigpipe): new.
+
 Sat May 28 16:08:16 2011  KOSAKI Motohiro  <kosaki.motohiro@g...>
 
 	* signal.c (Init_signal, default_handler): change default SIGPIPE handler
Index: process.c
===================================================================
--- process.c	(revision 31760)
+++ process.c	(revision 31761)
@@ -984,14 +984,41 @@
 
 static int forked_child = 0;
 
+#ifdef SIGPIPE
+static RETSIGTYPE (*saved_sigpipe_handler)(int) = 0;
+#endif
+
+#if defined(POSIX_SIGNAL)
+# define signal(a,b) posix_signal((a),(b))
+#endif
+
+static void save_sigpipe(void)
+{
+#ifdef SIGPIPE
+    /*
+     * Some OS commands don't initialize signal handler properly. Thus we have to
+     * reset signal handler before exec(). Otherwise, system() and similar child process
+     * interaction might fail. (e.g. ruby -e "system 'yes | ls'") [ruby-dev:12261]
+     */
+    saved_sigpipe_handler = signal(SIGPIPE, SIG_DFL);
+#endif
+}
+
+static void restore_sigpipe(void)
+{
+#ifdef SIGPIPE
+    signal(SIGPIPE, saved_sigpipe_handler);
+#endif
+}
+
 /*
  * On old MacOS X, exec() may return ENOTSUPP if the process have multiple threads.
  * Therefore we have to kill internal threads at once. [ruby-core: 10583]
  */
 #define before_exec() \
-    (rb_enable_interrupt(), (void)(forked_child ? 0 : (rb_thread_stop_timer_thread(), 1)))
+    (rb_enable_interrupt(), save_sigpipe(), (void)(forked_child ? 0 : (rb_thread_stop_timer_thread(), 1)))
 #define after_exec() \
-  (rb_thread_reset_timer_thread(), rb_thread_start_timer_thread(), forked_child = 0, rb_disable_interrupt())
+    (rb_thread_reset_timer_thread(), rb_thread_start_timer_thread(), forked_child = 0, restore_sigpipe(), rb_disable_interrupt())
 #define before_fork() before_exec()
 #define after_fork() (GET_THREAD()->thrown_errinfo = 0, after_exec())
 
@@ -2907,11 +2934,6 @@
     return Qnil;		/* not reached */
 }
 
-
-#if defined(POSIX_SIGNAL)
-# define signal(a,b) posix_signal((a),(b))
-#endif
-
 void
 rb_syswait(rb_pid_t pid)
 {
Index: test/ruby/test_process.rb
===================================================================
--- test/ruby/test_process.rb	(revision 31760)
+++ test/ruby/test_process.rb	(revision 31761)
@@ -1,6 +1,7 @@
 require 'test/unit'
 require 'tmpdir'
 require 'pathname'
+require 'timeout'
 require_relative 'envutil'
 require 'rbconfig'
 
@@ -1250,4 +1251,21 @@
     exs << Errno::E2BIG if defined?(Errno::E2BIG)
     assert_raise(*exs, bug4315) {Process.spawn('"a"|'*10_000_000)}
   end
+
+  def test_system_sigpipe
+    return if /mswin|mingw/ =~ RUBY_PLATFORM
+
+    pid = 0
+
+    with_tmpchdir do
+      assert_nothing_raised('[ruby-dev:12261]') do
+        timeout(3) do
+          pid = spawn('yes | ls')
+          Process.waitpid pid
+        end
+      end
+    end
+  ensure
+    Process.kill(:KILL, pid) if (pid != 0) rescue false
+  end
 end

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