I agree, adding a simple flush basically solves this issue. I look forward to hearing whether there's any good reason not to do so.
On Tue, Apr 6, 2021 at 3:38 PM Brian Burkhalter <brian.burkhal...@oracle.com> wrote: > > Hello, > > This does not sound unreasonable to me, although I do not know the historical > context of the design decisions involved, on which I expect that Alan Bateman > will comment tomorrow. > > As to the implementation, it might be just to modify > java.nio.channels.Channels$WritableByteChannelImpl.write() to flush the > wrapped stream in the sync block after the write loop. > > Brian > > On Apr 6, 2021, at 12:30 PM, Charles Oliver Nutter <head...@headius.com> > wrote: > > While debugging some subprocess logic in JRuby today I came to the > realization that the WritableByteChannel returned by > Channels.newChannel(OutputStream) is "broken". > > https://github.com/jruby/jruby/pull/6649/commits/5a6912caf3adb70fb4c210c73ae02fbf16f404d9 > > The issue arises when the provided stream is itself buffered, as is > the case for Process.getOutputStream. Because neither Channel nor > WritableByteChannel provide a way to flush (as Channels are expected > to be "raw" unbuffered IO), data written to the wrapper will not > propagate to the the other end of the stream until the intermediate > buffer has been filled. A flush can only be performed if you have > direct access to the underlying stream, which is generally impossible > if using this wrapper to interact with a Channel-related API. > > I would propose that the default OutputStream Channel wrapper should > flush() after every non-zero write (or potentially after every write, > if the underlying stream is not altogether honest about how many bytes > were written). > > Can someone provide a counter example for why the wrapper should *not* > flush (current behavior)? What would be harmed by making this change? > I can come up with nothing. > > Additional reasons to flush: > > * Early propagation of IO errors due to the underlying stream no > longer being flushable. > * More responsive, possibly reducing IO stutter for APIs using the wrapper. > > That first one could be seen as a negative but I can only see it as a > positive (raise the error at the time of the bad write, not once the > buffer is filled many writes later). > >
