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

Reopen stdio streams when run from Drip, to pick up its FIFOs. #4010

Merged
merged 2 commits into from Aug 11, 2016

Conversation

Projects
None yet
1 participant
@headius
Member

headius commented Jul 12, 2016

This fixes #2690, mostly.

Previous situation

On startup, we assume stdio will be at the usual 0,1,2 values descriptors, and we use those directly to open native stdio streams for Ruby. This conflicts with Drip background processes, since after the "drip main" has run, it switches System.in/out/err to point at FIFO file streams that are used to proxy the client process. If we assume stdio is always at 0,1,2, we wire up the wrong descriptors.

First fix

My first fix was to just skip the native stream logic and use our tried-and-true unwrapping logic to take System.in/out/err and unwrap them down to their innermost File*Stream. When running under Drip, one of its Switchable streams will be in the way, and we will fall back on using wrapped-stream logic for stdio. That fixes this issue at least well enough for stdio to function.

Unfortunately this fix has a problem: by not unwrapping all the way to the FileChannel, we lose some interactivity at a console. Granted, we already lose some interactivity due to Drip's FIFO being in the way, but on top of that we also have Java's stream buffering interfering with interactive use.

Additional fix

I continued with the above fix and added smarts to further unwrap any Switchable streams. After much fussing around, I have the logic properly getting Drip's FIFO file descriptors for stdio, and fully unwrapping them.

Unfortunately, by default when Drip boots JRuby, it attempts to use our org.jruby.main.DripMain to start up a background JRuby instance. This instance sees the stdio streams before they have been switched to the FIFOs, and so that prebooted instance has stdio pointing at the wrong descriptors.

We have addressed this for other Drip laziness by adding Ruby.reinitialize, which re-inits anything we need to defer until the process "really" starts. For example, ARGV is re-defined to point at the actual arguments passed in by the Drip client.

I also did this work so that the streams would be reinitialized, by replacing the existing descriptors in the stdio streams with the new ones from Drip. However, any code that grabs the Ruby stdio file descriptors before the "true" process start will continue to point at the old, incorrect descriptors.

Possible solutions

  • We could ignore the problem. Most of the time, people don't grab and hold onto the file descriptors for $stdin and friends...they just use top-level IO functions that write to those streams. But there's always troublemakers...
  • Perhaps we could persuade @ninjudd that the Drip daemon should actually dup the appropriate side of the FIFOs directly into stdio. This would mean every access from DripMain forward would see plain old stdio file descriptors without requiring any switching.

A side effect from my changes here is that stdio.fileno will now always reflect the actual fileno. Previously I forced stdio streams to always report their fileno in the 0,1,2 group, which doesn't make sense if they're not actually using those fileno.

@headius headius added this to the JRuby 9.1.3.0 milestone Jul 12, 2016

headius added some commits Jul 12, 2016

Reopen stdio streams when run from Drip, to pick up its FIFOs.
This fixes #2690, mostly.

Previous situation
==================

On startup, we assume stdio will be at the usual 0,1,2 values descriptors, and we use those directly to open native stdio streams for Ruby. This conflicts with Drip background processes, since after the "drip main" has run, it switches System.in/out/err to point at FIFO file streams that are used to proxy the client process. If we assume stdio is always at 0,1,2, we wire up the wrong descriptors.

First fix
=========

My first fix was to just skip the native stream logic and use our tried-and-true unwrapping logic to take System.in/out/err and unwrap them down to their innermost File*Stream. When running under Drip, one of its Switchable streams will be in the way, and we will fall back on using wrapped-stream logic for stdio. That fixes this issue at least well enough for stdio to function.

Unfortunately this fix has a problem: by not unwrapping all the way to the FileChannel, we lose some interactivity at a console. Granted, we already lose some interactivity due to Drip's FIFO being in the way, but on top of that we also have Java's stream buffering interfering with interactive use.

Additional fix
==============

I continued with the above fix and added smarts to further unwrap any Switchable streams. After much fussing around, I have the logic properly getting Drip's FIFO file descriptors for stdio, and fully unwrapping them.

Unfortunately, by default when Drip boots JRuby, it attempts to use our org.jruby.main.DripMain to start up a background JRuby instance. This instance sees the stdio streams *before* they have been switched to the FIFOs, and so that prebooted instance has stdio pointing at the wrong descriptors.

We have addressed this for other Drip laziness by adding Ruby.reinitialize, which re-inits anything we need to defer until the process "really" starts. For example, ARGV is re-defined to point at the actual arguments passed in by the Drip client.

I also did this work so that the streams would be reinitialized, by replacing the existing descriptors in the stdio streams with the new ones from Drip. However, any code that grabs the Ruby stdio file descriptors before the "true" process start will continue to point at the old, incorrect descriptors.

Possible solutions
==============

* We could ignore the problem. Most of the time, people don't grab and hold onto the file descriptors for `$stdin` and friends...they just use top-level IO functions that write to those streams. But there's always troublemakers...
* Perhaps we could persuade @ninjudd that the Drip daemon should actually dup the appropriate side of the FIFOs directly into stdio. This would mean every access from DripMain forward would see plain old stdio file descriptors without requiring any switching.

A side effect from my changes here is that stdio.fileno will now always reflect the actual fileno. Previously I forced stdio streams to always report their fileno in the 0,1,2 group, which doesn't make sense if they're not actually using those fileno.

@headius headius merged commit c059138 into jruby:master Aug 11, 2016

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@headius headius deleted the headius:reopen_stdio_for_drip branch Aug 11, 2016

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