From 0cebfad20ab9eb694eeb95f083635127cfbf35f9 Mon Sep 17 00:00:00 2001 From: kosaki Date: Tue, 30 Aug 2011 00:33:05 +0000 Subject: * 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 | 12 ++++ ext/-test-/old_thread_select/depend | 2 + ext/-test-/old_thread_select/extconf.rb | 1 + ext/-test-/old_thread_select/old_thread_select.c | 56 ++++++++++++++++++ .../old_thread_select/test_old_thread_select.rb | 69 ++++++++++++++++++++++ thread.c | 45 ++++++++------ 6 files changed, 166 insertions(+), 19 deletions(-) create mode 100644 ext/-test-/old_thread_select/depend create mode 100644 ext/-test-/old_thread_select/extconf.rb create mode 100644 ext/-test-/old_thread_select/old_thread_select.c create mode 100644 test/-ext-/old_thread_select/test_old_thread_select.rb diff --git a/ChangeLog b/ChangeLog index 2b6a8ebe47..46a03da6d2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +Tue Aug 30 09:28:01 2011 KOSAKI Motohiro + + * 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 * configure.in: fix a build failure on GNU Hurd. diff --git a/ext/-test-/old_thread_select/depend b/ext/-test-/old_thread_select/depend new file mode 100644 index 0000000000..a2bc836e1f --- /dev/null +++ b/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 diff --git a/ext/-test-/old_thread_select/extconf.rb b/ext/-test-/old_thread_select/extconf.rb new file mode 100644 index 0000000000..8b410218fa --- /dev/null +++ b/ext/-test-/old_thread_select/extconf.rb @@ -0,0 +1 @@ +create_makefile("-test-/old_thread_select/old_thread_select") diff --git a/ext/-test-/old_thread_select/old_thread_select.c b/ext/-test-/old_thread_select/old_thread_select.c new file mode 100644 index 0000000000..0c9cab58fd --- /dev/null +++ b/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); +} diff --git a/test/-ext-/old_thread_select/test_old_thread_select.rb b/test/-ext-/old_thread_select/test_old_thread_select.rb new file mode 100644 index 0000000000..886ee70cfa --- /dev/null +++ b/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 diff --git a/thread.c b/thread.c index 57a69629a6..2ec8b80265 100644 --- a/thread.c +++ b/thread.c @@ -2686,29 +2686,36 @@ int rb_thread_select(int max, fd_set * read, fd_set * write, fd_set * except, struct timeval *timeout) { - if (!read && !write && !except) { - if (!timeout) { - rb_thread_sleep_forever(); - return 0; - } - rb_thread_wait_for(*timeout); - return 0; + rb_fdset_t fdsets[3] = { 0 }; + rb_fdset_t *rfds = NULL; + rb_fdset_t *wfds = NULL; + rb_fdset_t *efds = NULL; + int retval; + + if (read) { + rfds = &fdsets[0]; + rb_fd_copy(rfds, read, max); + } + if (write) { + wfds = &fdsets[1]; + rb_fd_copy(wfds, write, max); + } + if (except) { + efds = &fdsets[2]; + rb_fd_copy(efds, except, max); } - else { - int lerrno = errno; - int result; - BLOCKING_REGION({ - result = select(max, read, write, except, timeout); - if (result < 0) - lerrno = errno; - }, ubf_select, GET_THREAD()); - errno = lerrno; + retval = rb_thread_fd_select(max, rfds, efds, wfds, timeout); - return result; - } -} + 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, -- cgit v1.2.3