* thread.c (rb_thread_select): rewrite by using

rb_thread_fd_select(). old one is EINTR unsafe.
  Patch by Eric Wong. [Bug #5229] [ruby-core:39102]

* test/-ext-/old_thread_select/test_old_thread_select.rb:
  a testcase for rb_thread_select().
* ext/-test-/old_thread_select/old_thread_select.c: ditto.
* ext/-test-/old_thread_select/depend: ditto.
* ext/-test-/old_thread_select/extconf.rb: ditto.



git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@33117 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

ChangeLog

@@ -1,3 +1,15 @@
+Tue Aug 30 09:28:01 2011  KOSAKI Motohiro  <kosaki.motohiro@gmail.com>
+
+	* thread.c (rb_thread_select): rewrite by using
+	  rb_thread_fd_select(). old one is EINTR unsafe.
+	  Patch by Eric Wong. [Bug #5229] [ruby-core:39102]
+	
+	* test/-ext-/old_thread_select/test_old_thread_select.rb:
+	  a testcase for rb_thread_select().
+	* ext/-test-/old_thread_select/old_thread_select.c: ditto.
+	* ext/-test-/old_thread_select/depend: ditto.
+	* ext/-test-/old_thread_select/extconf.rb: ditto.
+
 Tue Aug 30 09:08:22 2011  KOSAKI Motohiro  <kosaki.motohiro@gmail.com>
 
 	* configure.in: fix a build failure on GNU Hurd.

ext/-test-/old_thread_select/depend

@@ -0,0 +1,2 @@
+old_thread_select.o: $(top_srcdir)/thread.c \
+  $(hdrdir)/ruby/ruby.h $(hdrdir)/ruby/io.h

ext/-test-/old_thread_select/extconf.rb

@@ -0,0 +1 @@
+create_makefile("-test-/old_thread_select/old_thread_select")

ext/-test-/old_thread_select/old_thread_select.c

@@ -0,0 +1,56 @@
+/* test case for deprecated C API */
+#include "ruby/ruby.h"
+#include "ruby/io.h"
+
+static fd_set * array2fdset(fd_set *fds, VALUE ary, int *max)
+{
+    long i;
+
+    if (NIL_P(ary))
+	return NULL;
+
+    FD_ZERO(fds);
+    Check_Type(ary, T_ARRAY);
+    for (i = 0; i < RARRAY_LEN(ary); i++) {
+	VALUE val = RARRAY_PTR(ary)[i];
+	int fd;
+
+	Check_Type(val, T_FIXNUM);
+	fd = FIX2INT(val);
+	if (fd >= *max)
+	    *max = fd + 1;
+	FD_SET(fd, fds);
+    }
+
+    return fds;
+}
+
+static VALUE
+old_thread_select(VALUE klass, VALUE r, VALUE w, VALUE e, VALUE timeout)
+{
+    struct timeval tv;
+    struct timeval *tvp = NULL;
+    fd_set rfds, wfds, efds;
+    fd_set *rp, *wp, *ep;
+    int rc;
+    int max = 0;
+
+    if (!NIL_P(timeout)) {
+	tv = rb_time_timeval(timeout);
+	tvp = &tv;
+    }
+    rp = array2fdset(&rfds, r, &max);
+    wp = array2fdset(&wfds, w, &max);
+    ep = array2fdset(&efds, w, &max);
+    rc = rb_thread_select(max, rp, wp, ep, tvp);
+    if (rc == -1)
+	rb_sys_fail("rb_wait_for_single_fd");
+    return INT2NUM(rc);
+}
+
+void
+Init_old_thread_select(void)
+{
+    rb_define_singleton_method(rb_cIO, "old_thread_select",
+                               old_thread_select, 4);
+}

test/-ext-/old_thread_select/test_old_thread_select.rb

@@ -0,0 +1,69 @@
+require 'test/unit'
+
+class TestOldThreadSelect < Test::Unit::TestCase
+  require '-test-/old_thread_select/old_thread_select'
+
+  def with_pipe
+    r, w = IO.pipe
+    begin
+      yield r, w
+    ensure
+      r.close unless r.closed?
+      w.close unless w.closed?
+    end
+  end
+
+  def test_old_select_read_timeout
+    with_pipe do |r, w|
+      t0 = Time.now
+      rc = IO.old_thread_select([r.fileno], nil, nil, 0.001)
+      diff = Time.now - t0
+      assert_equal 0, rc
+      assert diff > 0.001, "returned too early"
+    end
+  end
+
+  def test_old_select_read_write_check
+    with_pipe do |r, w|
+      w.syswrite('.')
+      rc = IO.old_thread_select([r.fileno], nil, nil, nil)
+      assert_equal 1, rc
+
+      rc = IO.old_thread_select([r.fileno], [w.fileno], nil, nil)
+      assert_equal 2, rc
+
+      assert_equal '.', r.read(1)
+
+      rc = IO.old_thread_select([r.fileno], [w.fileno], nil, nil)
+      assert_equal 1, rc
+    end
+  end
+
+  def test_old_select_signal_safe
+    return unless Process.respond_to?(:kill)
+    usr1 = false
+    trap(:USR1) { usr1 = true }
+    main = Thread.current
+    thr = Thread.new do
+      Thread.pass until main.stop?
+      Process.kill(:USR1, $$)
+      true
+    end
+
+    rc = nil
+    t0 = Time.now
+    with_pipe do |r,w|
+      assert_nothing_raised do
+        rc = IO.old_thread_select([r.fileno], nil, nil, 1)
+      end
+    end
+
+    diff = Time.now - t0
+    assert diff >= 1.0, "interrupted or short wait"
+    assert_equal 0, rc
+    assert_equal true, thr.value
+    assert usr1, "USR1 not received"
+    ensure
+      trap(:USR1, "DEFAULT")
+  end
+end

thread.c

@@ -2686,30 +2686,37 @@ int
 rb_thread_select(int max, fd_set * read, fd_set * write, fd_set * except,
 		 struct timeval *timeout)
 {
-    if (!read && !write && !except) {
+    rb_fdset_t fdsets[3] = { 0 };
-	if (!timeout) {
+    rb_fdset_t *rfds = NULL;
-	    rb_thread_sleep_forever();
+    rb_fdset_t *wfds = NULL;
-	    return 0;
+    rb_fdset_t *efds = NULL;
-	}
+    int retval;
-	rb_thread_wait_for(*timeout);
-	return 0;
-    }
-    else {
-	int lerrno = errno;
-	int result;
 
-	BLOCKING_REGION({
+    if (read) {
-		result = select(max, read, write, except, timeout);
+	rfds = &fdsets[0];
-		if (result < 0)
+	rb_fd_copy(rfds, read, max);
-		    lerrno = errno;
-	    }, ubf_select, GET_THREAD());
-	errno = lerrno;
-
-	return result;
     }
+    if (write) {
+	wfds = &fdsets[1];
+	rb_fd_copy(wfds, write, max);
+    }
+    if (except) {
+	efds = &fdsets[2];
+	rb_fd_copy(efds, except, max);
+    }
+
+    retval = rb_thread_fd_select(max, rfds, efds, wfds, timeout);
+
+    if (rfds)
+	rb_fd_term(rfds);
+    if (wfds)
+	rb_fd_term(wfds);
+    if (efds)
+	rb_fd_term(efds);
+
+    return retval;
 }
 
-
 int
 rb_thread_fd_select(int max, rb_fdset_t * read, rb_fdset_t * write, rb_fdset_t * except,
 		    struct timeval *timeout)