Bug #6629


[PATCH] io.c: avoid rb_thread_wait_fd() if we may call rb_io_wait_readable()

Added by normalperson (Eric Wong) about 10 years ago. Updated almost 10 years ago.

Target version:
ruby -v:
ruby 2.0.0dev (2012-06-23 trunk 36192) [x86_64-linux]


Blindly calling rb_thread_wait_fd() is an extra, unnecessary
system call in some cases. Since we already call
rb_io_wait_readable() when encountering EAGAIN, there is no
user-visible change in behavior and a small potential for
speedup by avoiding a system call (especially on regular
filesystem that should never return EAGAIN/EINTR).

This also helps avoid triggering a bugs on some buggy (ancient)
Linux kernels. I encountered this issue[1] on a CentOS 5.7
machine running the 2.6.18-274.7.1.el5 kernel. Upgrading to
the 2.6.18-308.8.2.el5 kernel fixes the issue.

This Linux kernel regression I encountered was introduced with
commit 786d7e1612f0b0adb6046f19b906609e4fe8b1ba (Jul 15 2007)
and fixed in
commit dd23aae4f5edf4e1dbd8f7f8013a754ba3253f48 (Sep 11 2007)
I've verified the dd23aae4f5edf4e1dbd8f7f8013a754ba3253f48
change is present in the 2.6.18-308.8.2.el5 (working) kernel
but not in the 2.6.18-274.7.1.el5 (broken) kernel.

I realize this is a 5 year-old (fixed) bug in Linux, but I also
believe Ruby is is wrong to call select()/ppoll() before
encountering EAGAIN.


Note: I left IO#sysread unchanged (with just a comment) since
removing rb_thread_wait_fd() there would change semantics
(as would handling EAGAIN explicitly).


Updated by mame (Yusuke Endoh) about 10 years ago

  • Status changed from Open to Assigned
  • Assignee set to kosaki (Motohiro KOSAKI)
Actions #2

Updated by kosaki (Motohiro KOSAKI) almost 10 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r36931.
Eric, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

  • io.c (io_bufread): removed unnecessary rb_thread_wait_fd().
    Patch by Eric Wong. [Bug #6629] [ruby-core:45789]
  • io.c (rb_io_sysread): ditto.
  • io.c (copy_stream_fallback_body): ditto.

Also available in: Atom PDF