From 537e4c341137a45330e28376e8f29da7df44808f Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 4 May 2011 19:26:38 -0700 Subject: Kgio.poll restarts on interrupt This changes our semantics, but it's unlikely anybody relies on EINTR right now... --- ext/kgio/broken_system_compat.h | 62 +++++++++++++++++++++++++++++++++++++++++ ext/kgio/extconf.rb | 7 ++++- ext/kgio/poll.c | 55 +++++++++++++++++++++++++++++++++++- test/test_poll.rb | 29 +++++++++++-------- 4 files changed, 139 insertions(+), 14 deletions(-) create mode 100644 ext/kgio/broken_system_compat.h diff --git a/ext/kgio/broken_system_compat.h b/ext/kgio/broken_system_compat.h new file mode 100644 index 0000000..e67a56c --- /dev/null +++ b/ext/kgio/broken_system_compat.h @@ -0,0 +1,62 @@ +/* + * this header includes functions to support broken systems + * without clock_gettime() or CLOCK_MONOTONIC + */ + +#ifndef HAVE_TYPE_CLOCKID_T +typedef int clockid_t; +#endif + +#ifndef HAVE_CLOCK_GETTIME +# ifndef CLOCK_REALTIME +# define CLOCK_REALTIME 0 /* whatever */ +# endif +static int fake_clock_gettime(clockid_t clk_id, struct timespec *res) +{ + struct timeval tv; + int r = gettimeofday(&tv, NULL); + + assert(0 == r && "gettimeofday() broke!?"); + res->tv_sec = tv.tv_sec; + res->tv_nsec = tv.tv_usec * 1000; + + return r; +} +# define clock_gettime fake_clock_gettime +#endif /* broken systems w/o clock_gettime() */ + +/* + * UGH + * CLOCK_MONOTONIC is not guaranteed to be a macro, either + */ +#ifndef CLOCK_MONOTONIC +# if (!defined(_POSIX_MONOTONIC_CLOCK) || !defined(HAVE_CLOCK_MONOTONIC)) +# define CLOCK_MONOTONIC CLOCK_REALTIME +# endif +#endif + +/* + * Availability of a monotonic clock needs to be detected at runtime + * since we could've been built on a different system than we're run + * under. + */ +static clockid_t hopefully_CLOCK_MONOTONIC; + +static int check_clock(void) +{ + struct timespec now; + + hopefully_CLOCK_MONOTONIC = CLOCK_MONOTONIC; + + /* we can't check this reliably at compile time */ + if (clock_gettime(CLOCK_MONOTONIC, &now) == 0) + return 1; + + if (clock_gettime(CLOCK_REALTIME, &now) == 0) { + hopefully_CLOCK_MONOTONIC = CLOCK_REALTIME; + rb_warn("CLOCK_MONOTONIC not available, " + "falling back to CLOCK_REALTIME"); + return 2; + } + return -1; +} diff --git a/ext/kgio/extconf.rb b/ext/kgio/extconf.rb index 5beb247..f44b8c8 100644 --- a/ext/kgio/extconf.rb +++ b/ext/kgio/extconf.rb @@ -1,7 +1,12 @@ require 'mkmf' $CPPFLAGS << ' -D_GNU_SOURCE' $CPPFLAGS << ' -DPOSIX_C_SOURCE=1' - +$CPPFLAGS += '-D_POSIX_C_SOURCE=200112L' +unless have_macro('CLOCK_MONOTONIC', 'time.h') + have_func('CLOCK_MONOTONIC', 'time.h') +end +have_type('clockid_t', 'time.h') +have_library('rt', 'clock_gettime', 'time.h') have_func("poll", "poll.h") have_func("getaddrinfo", %w(sys/types.h sys/socket.h netdb.h)) or abort "getaddrinfo required" diff --git a/ext/kgio/poll.c b/ext/kgio/poll.c index 502f660..614d939 100644 --- a/ext/kgio/poll.c +++ b/ext/kgio/poll.c @@ -1,6 +1,9 @@ #include "kgio.h" #if defined(USE_KGIO_POLL) +#include +#include "broken_system_compat.h" #include +#include #ifdef HAVE_RUBY_ST_H # include #else @@ -16,8 +19,43 @@ struct poll_args { int timeout; VALUE ios; st_table *fd_to_io; + struct timespec start; }; +static int interrupted(void) +{ + switch (errno) { + case EINTR: +#ifdef ERESTART + case ERESTART: +#endif + return 1; + } + return 0; +} + +static int retryable(struct poll_args *a) +{ + struct timespec ts; + + if (a->timeout < 0) + return 1; + if (a->timeout == 0) + return 0; + + clock_gettime(hopefully_CLOCK_MONOTONIC, &ts); + + ts.tv_sec -= a->start.tv_sec; + ts.tv_nsec -= a->start.tv_nsec; + if (ts.tv_nsec < 0) { + ts.tv_sec--; + ts.tv_nsec += 1000000000; + } + a->timeout -= ts.tv_sec * 1000; + a->timeout -= ts.tv_nsec / 1000000; + return (a->timeout >= 0); +} + static int num2timeout(VALUE timeout) { switch (TYPE(timeout)) { @@ -70,6 +108,10 @@ static void hash2pollfds(struct poll_args *a) static VALUE nogvl_poll(void *ptr) { struct poll_args *a = ptr; + + if (a->timeout > 0) + clock_gettime(hopefully_CLOCK_MONOTONIC, &a->start); + return (VALUE)poll(a->fds, a->nfds, a->timeout); } @@ -100,8 +142,16 @@ static VALUE do_poll(VALUE args) Check_Type(a->ios, T_HASH); hash2pollfds(a); +retry: nr = (int)rb_thread_blocking_region(nogvl_poll, a, RUBY_UBF_IO, NULL); - if (nr < 0) rb_sys_fail("poll"); + if (nr < 0) { + if (interrupted()) { + if (retryable(a)) + goto retry; + return Qnil; + } + rb_sys_fail("poll"); + } if (nr == 0) return Qnil; return poll_result(nr, a); @@ -156,6 +206,9 @@ static VALUE s_poll(int argc, VALUE *argv, VALUE self) void init_kgio_poll(void) { VALUE mKgio = rb_define_module("Kgio"); + + if (check_clock() < 0) + return; rb_define_singleton_method(mKgio, "poll", s_poll, -1); sym_wait_readable = ID2SYM(rb_intern("wait_readable")); diff --git a/test/test_poll.rb b/test/test_poll.rb index 8cade91..1c92223 100644 --- a/test/test_poll.rb +++ b/test/test_poll.rb @@ -42,27 +42,32 @@ class TestPoll < Test::Unit::TestCase assert_nil res end - def test_poll_interrupt + def test_poll_close foo = nil - oldquit = trap(:QUIT) { foo = :bar } - thr = Thread.new { sleep 0.100; Process.kill(:QUIT, $$) } + thr = Thread.new { sleep 0.100; @wr.close } t0 = Time.now - assert_raises(Errno::EINTR) { Kgio.poll({}) } + res = Kgio.poll({@rd => Kgio::POLLIN}) diff = Time.now - t0 thr.join + assert_equal([ @rd ], res.keys) assert diff >= 0.010, "diff=#{diff}" - ensure - trap(:QUIT, "DEFAULT") end - def test_poll_close - foo = nil - thr = Thread.new { sleep 0.100; @wr.close } + def test_poll_EINTR + ok = false + orig = trap(:USR1) { ok = true } + thr = Thread.new do + sleep 0.100 + Process.kill(:USR1, $$) + end t0 = Time.now - res = Kgio.poll({@rd => Kgio::POLLIN}) + res = Kgio.poll({@rd => Kgio::POLLIN}, 1000) diff = Time.now - t0 thr.join - assert_equal([ @rd ], res.keys) - assert diff >= 0.010, "diff=#{diff}" + assert_nil res + assert diff >= 1.0, "diff=#{diff}" + assert ok + ensure + trap(:USR1, orig) end end if Kgio.respond_to?(:poll) -- cgit v1.2.3-24-ge0c7