Permalink
Browse files

Fixed regression in JRUBY-1923: STDIN not working under Kernel.system,

after IO reorg.

Also, added few unit tests.


git-svn-id: http://svn.codehaus.org/jruby/trunk/jruby@5909 961051c9-f516-0410-bf72-c9f7e237a7b7
  • Loading branch information...
1 parent 865b6fe commit 3c58a8a75d242208c063fb4d9c850f803c98b667 @vvs vvs committed Feb 11, 2008
@@ -245,7 +245,8 @@ public RubyIO(Ruby runtime, STDIO stdio) {
openFile.setMainStream(
new ChannelStream(
runtime,
- new ChannelDescriptor(Channels.newChannel(runtime.getIn()), 0, new ModeFlags(ModeFlags.RDONLY), FileDescriptor.in),
+ // special constructor that accepts stream, not channel
+ new ChannelDescriptor(runtime.getIn(), 0, new ModeFlags(ModeFlags.RDONLY), FileDescriptor.in),
FileDescriptor.in));
break;
case OUT:
@@ -286,7 +286,12 @@ private void handleStreams(Process p, InputStream in, OutputStream out, OutputSt
StreamPumper t1 = new StreamPumper(pOut, out, false);
StreamPumper t2 = new StreamPumper(pErr, err, false);
+
+ // The assumption here is that the 'in' stream provides
+ // proper available() support. If available() always
+ // returns 0, we'll hang!
StreamPumper t3 = new StreamPumper(in, pIn, true);
+
t1.start();
t2.start();
t3.start();
@@ -91,6 +91,13 @@
* Only counts references through ChannelDescriptor instances.
*/
private final AtomicInteger refCounter;
+
+ /**
+ * Used to work-around blocking problems with STDIN. In most cases <code>null</code>.
+ * See {@link #ChannelDescriptor(InputStream, int, ModeFlags, FileDescriptor)}
+ * for more details. You probably should not use it.
+ */
+ private InputStream baseInputStream;
/**
* Construct a new ChannelDescriptor with the specified channel, file number,
@@ -114,7 +121,7 @@ private ChannelDescriptor(Channel channel, int fileno, ModeFlags originalModes,
}
/**
- * Construct a new ChannelDescriptor with the given channel, fil enumber, mode flags,
+ * Construct a new ChannelDescriptor with the given channel, file number, mode flags,
* and file descriptor object. The channel will be kept open until all ChannelDescriptor
* references to it have been closed.
*
@@ -126,9 +133,31 @@ private ChannelDescriptor(Channel channel, int fileno, ModeFlags originalModes,
public ChannelDescriptor(Channel channel, int fileno, ModeFlags originalModes, FileDescriptor fileDescriptor) {
this(channel, fileno, originalModes, fileDescriptor, new AtomicInteger(1));
}
-
+
+ /**
+ * Special constructor to create the ChannelDescriptor out of the stream, file number,
+ * mode flags, and file descriptor object. The channel will be created from the
+ * provided stream. The channel will be kept open until all ChannelDescriptor
+ * references to it have been closed. <b>Note:</b> in most cases, you should not
+ * use this constructor, it's reserved mostly for STDIN.
+ *
+ * @param baseInputStream The stream to create the channel for the new descriptor
+ * @param fileno The file number for the new descriptor
+ * @param originalModes The mode flags for the new descriptor
+ * @param fileDescriptor The java.io.FileDescriptor object for the new descriptor
+ */
+ public ChannelDescriptor(InputStream baseInputStream, int fileno, ModeFlags originalModes, FileDescriptor fileDescriptor) {
+ // The reason why we need the stream is to be able to invoke available() on it.
+ // STDIN in Java is non-interruptible, non-selectable, and attempt to read
+ // on such stream might lead to thread being blocked without *any* way to unblock it.
+ // That's where available() comes it, so at least we could check whether
+ // anything is available to be read without blocking.
+ this(Channels.newChannel(baseInputStream), fileno, originalModes, fileDescriptor, new AtomicInteger(1));
+ this.baseInputStream = baseInputStream;
+ }
+
/**
- * Construct a new ChannelDescriptor with the given channel, fil enumber,
+ * Construct a new ChannelDescriptor with the given channel, file number,
* and file descriptor object. The channel will be kept open until all ChannelDescriptor
* references to it have been closed. The channel's capabilities will be used
* to determine the "original" set of mode flags.
@@ -175,6 +204,16 @@ public Channel getChannel() {
}
/**
+ * This is intentionally non-public, since it should not be really
+ * used outside of very limited use case (handling of STDIN).
+ * See {@link #ChannelDescriptor(InputStream, int, ModeFlags, FileDescriptor)}
+ * for more info.
+ */
+ /*package-protected*/ InputStream getBaseInputStream() {
+ return baseInputStream;
+ }
+
+ /**
* Whether the channel associated with this descriptor is seekable (i.e.
* whether it is instanceof FileChannel).
*
@@ -350,7 +350,12 @@ private void flushWrite() throws IOException, BadDescriptorException {
* @see org.jruby.util.IOHandler#getInputStream()
*/
public InputStream newInputStream() {
- return new BufferedInputStream(Channels.newInputStream((ReadableByteChannel)descriptor.getChannel()));
+ InputStream in = descriptor.getBaseInputStream();
+ if (in == null) {
+ return new BufferedInputStream(Channels.newInputStream((ReadableByteChannel)descriptor.getChannel()));
+ } else {
+ return in;
+ }
}
/**
@@ -3,6 +3,7 @@
require 'jruby'
class TestLaunchingByShellScript < Test::Unit::TestCase
+ WINDOWS = Config::CONFIG['host_os'] =~ /Windows|mswin/
RUBY = File.join(Config::CONFIG['bindir'], Config::CONFIG['ruby_install_name'])
def jruby(*args)
prev_in_process = JRuby.runtime.instance_config.run_ruby_in_process
@@ -12,6 +13,14 @@ def jruby(*args)
JRuby.runtime.instance_config.run_ruby_in_process = prev_in_process
end
+ def jruby_with_pipe(pipe, *args)
+ prev_in_process = JRuby.runtime.instance_config.run_ruby_in_process
+ JRuby.runtime.instance_config.run_ruby_in_process = false
+ `#{pipe} | #{RUBY} #{args.join(' ')}`
+ ensure
+ JRuby.runtime.instance_config.run_ruby_in_process = prev_in_process
+ end
+
def test_minus_e
assert_equal "true", jruby('-e "puts true"').chomp
assert_equal 0, $?.exitstatus
@@ -22,6 +31,18 @@ def test_launch_script
assert_equal 0, $?.exitstatus
end
+ def test_system_call_without_stdin_data_doesnt_hang
+ out = jruby("-e 'system \"dir\"'")
+ assert(out =~ /COPYING.LGPL/)
+ end
+
+ if !WINDOWS
+ def test_system_call_with_stdin_data_doesnt_hang
+ out = jruby_with_pipe("echo 'vvs'", "-e 'system \"cat\"'")
+ assert_equal("vvs\n", out)
+ end
+ end
+
def test_at_exit
assert_equal "", jruby("-e 'at_exit { exit 0 }'").chomp
assert_equal 0, $?.exitstatus

0 comments on commit 3c58a8a

Please sign in to comment.