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

• Status: Closed
• Assignee: kosaki (Motohiro KOSAKI)
• Target version: -
• ruby -v: ruby 2.0.0dev (2012-06-23 trunk 36192) [x86_64-linux]

[normalperson (Eric Wong)]

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.

[1] http://mid.gmane.org/[e-mail]

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).

[mame (Yusuke Endoh)]

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

[kosaki (Motohiro KOSAKI)]

• 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.