Faulty Null-Check Suspected in ToolProvider

[Philipp Kunz]

Hi Jon,

On Sat, 2019-02-16 at 13:44 -0800, Jonathan Gibbons wrote:
> On 2/16/19 12:20 AM, Philipp Kunz wrote:
> > I'm also wondering about the call to flush in run(PrintStream out,
> > PrintStream err, String... args). It looks like the intention was
> > to
> > flush the wrapping PrintWriter.
> > That is not possible without also flushing the underlying
> > PrintStream.
> > BufferedWriter.flushBuffer would be a more sensible method to call
> > but
> > is not accessible.
> > The effect is actually, that the call to PrintWriter.flush will
> > also
> > call flush of the underlying PrintStream. Should that be documented
> > more explicitly, for example:
> 
> Philipp,
> 
> I don't see that it needs to be specified.
> It is a reasonable presumption that everything written by the tool
> is propagated to the streams passed into the run method. How that
> is achieved is an implementation detail.

I absolutely agree so far. My point, however, was a different one.
In addition to flushing the buffers created by run(PrintStream,
PrintStream, String...)'s PrintWriters to the PrintStreams passed to
that method, flushing the PrintWriters propagates to the underlying
PrintStreams and flushes them as well.
In other words, whatever PrintStreams passed into run(PrintStream,
PrintStream, String...) are flushed.

> If you were to modify the spec, it would at most be an implementation
> detail, and should appear in an @implNote.

I agree that it should go into an @implNote. I also suggested it that
way, below an existing @implNote.


> -- Jon
> 

Additionally, run(PrintStream, PrintStream, String...) is only a
default method and may be replaced in ToolProvider implementations.
Therefore we don't know if these buffers will actually be flushed and I
replaced "are" with "may be" before "flushed" in the comment.
The same consideration also applies to run(PrintWriter, PrintWriter,
String...).


diff -r 31e3aa9c0c71
src/java.base/share/classes/java/util/spi/ToolProvider.java
--- a/src/java.base/share/classes/java/util/spi/ToolProvider.java	
Sat Feb 16 11:40:34 2019 +0900
+++ b/src/java.base/share/classes/java/util/spi/ToolProvider.java	
Sun Feb 17 01:30:30 2019 +0100
@@ -75,6 +75,8 @@
      * @apiNote The interpretation of the arguments will be specific
to
      * each tool.
      *
+     * @implNote Both {@code out} and {@code err} streams may be
flushed.
+     *
      * @param out a stream to which "expected" output should be
written
      *
      * @param err a stream to which any error messages should be
written
@@ -107,6 +109,8 @@
      * @implNote This implementation wraps the {@code out} and {@code
err}
      * streams within {@link PrintWriter}s, and then calls
      * {@link #run(PrintWriter, PrintWriter, String[])}.
+     * Both {@code out} and {@code err} streams may be flushed before
the method
+     * returns as a side-effect of flushing the wrapping {@link
PrintWriter}s.
      *
      * @param out a stream to which "expected" output should be
written
      *


Regards,
Philipp