Project

General

Profile

Bug #10494

ioctl returns EINVAL instead of ENOTTY for pipes on older linux, breaking piped test suite

Added by headius (Charles Nutter) about 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
ruby -v:
ruby 2.1.4p265 (2014-10-27 revision 48166) [x86_64-linux]
[ruby-core:66191]

Description

Yeah I know the title is a mouthful. I'll try to explain.

BACKGROUND:

Starting sometime recently, MRI's test suite started to use a custom version of test/unit contained in test/lib. Inside unit.rb, there's code that uses io/console to determine the window size for some fancy formatting.

https://github.com/ruby/ruby/blob/trunk/test/lib/test/unit.rb#L404

The surrounding rescue captures LoadError (io/console doesn't exist), NoMethodError (winsize is not defined), ENOTTY (fd is not a tty), and EBADF (fd is no longer valid), and provides suitable fallback code for console width.

PROBLEM:

However, when we ran the MRI suite with JRuby on Travis, we got the following error:

 [exec] Run options: -q --
 [exec] 
 [exec] # Running tests:
 [exec] 
 [exec] 
 [exec] Errno::EINVAL: Invalid argument - ioctl(TIOCGWINSZ)         winsize at /home/travis/build/jruby/jruby/lib/ruby/stdlib/io/console.rb:120
 [exec]   terminal_width at /home/travis/build/jruby/jruby/test/mri/lib/test/unit.rb:404
 [exec]       put_status at /home/travis/build/jruby/jruby/test/mri/lib/test/unit.rb:433

This is from within a Maven build target we use to run our test suites with the proper environment. We could not reproduce this on current Linux versions.

CAUSE:

For those unfamiliar with the JVM, the standard process-launching APIs always set the child's stdio to pipes that you can then read, a la popen in MRI. Long story short, it turns out that Linux incorrectly returned EINVAL instead of ENOTTY when calling ioctl with a pipe up until this fix in May of 2012: https://lkml.org/lkml/2012/5/25/142

This is after the release of Ubuntu 12.04 that Travis runs, and I'm guessing they have not updated the kernels.

REPRODUCTION (for Linux kernels prior to this patch):

I was able to reproduce this EINVAL on a Travis dev box (thanks asarih!) with both JRuby and MRI using the following command line:

$ rvm ruby-2.1 do ruby -rio/console -e 'p $stdout.winsize' | cat
-e:1:in `winsize': Invalid argument (Errno::EINVAL)
    from -e:1:in `<main>'

SOLUTION:

????????

I'm not sure.

  • We could just patch the test library. I have done this for JRuby here: https://github.com/jruby/jruby/commit/29122372da5ba2f359239aa916e074a57fe96b70

  • We could modify io/console #winsize and other methods to raise this incorrect EINVAL as ENOTTY, since we should know in each of those methods that we have the right request and data structure (ergo EINVAL should not happen on non-buggy systems)

We need to do one or the other for the test suite to be runnable on these earlier Linux kernels when stdout is a pipe.

Associated revisions

Revision 0a528213
Added by headius (Charles Nutter) almost 4 years ago

  • test/lib/test/unit.rb: Also rescue EINVAL for older Linux that raises it in popen. [Bug #10494]

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

Revision 48854
Added by headius (Charles Nutter) almost 4 years ago

  • test/lib/test/unit.rb: Also rescue EINVAL for older Linux that raises it in popen. [Bug #10494]

Revision 48854
Added by headius (Charles Nutter) almost 4 years ago

  • test/lib/test/unit.rb: Also rescue EINVAL for older Linux that raises it in popen. [Bug #10494]

Revision 48854
Added by headius (Charles Nutter) almost 4 years ago

  • test/lib/test/unit.rb: Also rescue EINVAL for older Linux that raises it in popen. [Bug #10494]

Revision 48854
Added by headius (Charles Nutter) almost 4 years ago

  • test/lib/test/unit.rb: Also rescue EINVAL for older Linux that raises it in popen. [Bug #10494]

Revision 48854
Added by headius (Charles Nutter) almost 4 years ago

  • test/lib/test/unit.rb: Also rescue EINVAL for older Linux that raises it in popen. [Bug #10494]

Revision 11ab6f8c
Added by usa (Usaku NAKAMURA) almost 4 years ago

merge revision(s) 48854: [Backport #10494]

* test/lib/test/unit.rb: Also rescue EINVAL for older Linux that

raises it in popen. [Bug #10494]

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

Revision 49446
Added by usa (Usaku NAKAMURA) almost 4 years ago

merge revision(s) 48854: [Backport #10494]

* test/lib/test/unit.rb: Also rescue EINVAL for older Linux that

raises it in popen. [Bug #10494]

Revision 38117a85
Added by nagachika (Tomoyuki Chikanaga) almost 4 years ago

merge revision(s) r48854: [Backport #10494]

* test/lib/test/unit.rb: Also rescue EINVAL for older Linux that

raises it in popen. [Bug #10494]

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

Revision 49910
Added by nagachika (Tomoyuki Chikanaga) almost 4 years ago

merge revision(s) r48854: [Backport #10494]

* test/lib/test/unit.rb: Also rescue EINVAL for older Linux that

raises it in popen. [Bug #10494]

History

#1 [ruby-core:66198] Updated by headius (Charles Nutter) about 4 years ago

I have reported to Travis that their kernel may be missing this patch. Not directly related to the outcome of this bug, but perhaps there will soon be fewer places to encounter it.

#2 [ruby-core:66859] Updated by headius (Charles Nutter) almost 4 years ago

If there are no objections I'm going to go ahead and patch test/lib/test/unit.rb the same way as we did it in JRuby.

#3 [ruby-core:66860] Updated by headius (Charles Nutter) almost 4 years ago

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

Applied in changeset r48854.


  • test/lib/test/unit.rb: Also rescue EINVAL for older Linux that raises it in popen. [Bug #10494]

#4 Updated by usa (Usaku NAKAMURA) almost 4 years ago

  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN to 2.0.0: REQUIRED, 2.1: REQUIRED

#5 [ruby-core:67899] Updated by usa (Usaku NAKAMURA) almost 4 years ago

  • Backport changed from 2.0.0: REQUIRED, 2.1: REQUIRED to 2.0.0: DONE, 2.1: REQUIRED

ruby_2_0_0 r49446 merged revision(s) 48854.
Note that ruby_2_0_0 has lib/test/unit.rb instead of at test/lib.

#6 [ruby-core:68469] Updated by nagachika (Tomoyuki Chikanaga) almost 4 years ago

  • Backport changed from 2.0.0: DONE, 2.1: REQUIRED to 2.0.0: DONE, 2.1: DONE

Backported into ruby_2_1 branch at r49910.

Also available in: Atom PDF