New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

popen doesn't work on Windows #4843

Closed
yamam opened this Issue Nov 9, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@yamam

yamam commented Nov 9, 2017

Environment

Provide at least:
Windows 10 64bit
jruby 9.1.14.0 (2.3.3) 2017-11-08 2176f24 Java HotSpot(TM) Client VM 25.144-b01 on 1.8.0_144-b01 +jit [mswin32-x86]

Expected Behavior

In jruby 9.1.13, this command outputs "a"

/cygdrive/c/jruby-9.1.13.0/bin/jruby.exe -e 'io = IO.popen("cat", "r+"); io.puts("a");p io.read(1)'
"a"

Actual Behavior

In jruby 9.1.14, no return

/cygdrive/c/jruby-9.1.14.0/bin/jruby.exe -e 'io = IO.popen("cat", "r+"); io.puts("a");p io.read(1)'

On Linux, it returns "a"

./jruby-9.1.14.0/bin/jruby -e 'io = IO.popen("cat", "r+"); io.puts("a");p io.read(1)'                                                                         
"a"
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

I assume there was some copy/paste problem here? You're showing 9.1.13 failing to return "a" above.

Member

headius commented Nov 9, 2017

I assume there was some copy/paste problem here? You're showing 9.1.13 failing to return "a" above.

@headius headius added this to the JRuby 9.1.15.0 milestone Nov 9, 2017

@headius headius added the core label Nov 9, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

Maybe related, passing -Xnative.enabled=false makes this hang on macos. Does it hang for you on Windows or just not display anything?

Member

headius commented Nov 9, 2017

Maybe related, passing -Xnative.enabled=false makes this hang on macos. Does it hang for you on Windows or just not display anything?

@yamam

This comment has been minimized.

Show comment
Hide comment
@yamam

yamam Nov 9, 2017

I assume there was some copy/paste problem here? You're showing 9.1.13 failing to return "a" above.

I fixed it.

Does it hang for you on Windows or just not display anything?

It hangs.

yamam commented Nov 9, 2017

I assume there was some copy/paste problem here? You're showing 9.1.13 failing to return "a" above.

I fixed it.

Does it hang for you on Windows or just not display anything?

It hangs.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

Ok, well my first suspect turned out to be right: I can see the data is buffered in the JDK-provided streams. This means one of two things: we were not able to unwrap the buffering, or chose not to for various reasons.

Some versions of JDK (I think 7+, unfortunately) now drive the subprocess streams by themselves, constantly draining them. This makes true interactive subprocesses impossible, and along with them many uses of Ruby. On UNIX platforms we have worked around this by implementing our own process logic that uses posix_spawn and achieves good compatibility. However, on Windows, process management is more complicated, and we have not yet had the resources to mimic the logic in MRI.

I do not know why this would have changed from 9.1.13 to 9.1.14, however. I'll review logs for changes to process and IO logic and see if something jumps out.

Member

headius commented Nov 9, 2017

Ok, well my first suspect turned out to be right: I can see the data is buffered in the JDK-provided streams. This means one of two things: we were not able to unwrap the buffering, or chose not to for various reasons.

Some versions of JDK (I think 7+, unfortunately) now drive the subprocess streams by themselves, constantly draining them. This makes true interactive subprocesses impossible, and along with them many uses of Ruby. On UNIX platforms we have worked around this by implementing our own process logic that uses posix_spawn and achieves good compatibility. However, on Windows, process management is more complicated, and we have not yet had the resources to mimic the logic in MRI.

I do not know why this would have changed from 9.1.13 to 9.1.14, however. I'll review logs for changes to process and IO logic and see if something jumps out.

@headius headius closed this in 632c2f1 Nov 9, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

This is fixed. It appears to have been a copy/paste bug, using the wrong type of stream for reflection during stream unwrapping.

My best recommendation for the moment would be to stick to 9.1.13 until we can spin a 9.1.15 release. That will likely happen in a couple weeks, after RubyConf.

Member

headius commented Nov 9, 2017

This is fixed. It appears to have been a copy/paste bug, using the wrong type of stream for reflection during stream unwrapping.

My best recommendation for the moment would be to stick to 9.1.13 until we can spin a 9.1.15 release. That will likely happen in a couple weeks, after RubyConf.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 9, 2017

Member

cc @enebo for thoughts on release timeframe and impact.

Member

headius commented Nov 9, 2017

cc @enebo for thoughts on release timeframe and impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment