Why doesn't Channels.newChannel(OutputStream) flush?

[Charles Oliver Nutter]

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.burkhalter at 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 <headius at 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).
>
>