RFR: CODETOOLS-7902337 jcov (JREInstr) hides results of running java commands: jlink, jimage

[Leonid Kuskov]

Jon,

Thank you for so detailed review and comments. I've updated the method: 
http://cr.openjdk.java.net/~lkuskov/7902337/webrev.02

Please take a look at the issue: 
https://bugs.openjdk.java.net/browse/CODETOOLS-7902338
and the fix: http://cr.openjdk.java.net/~lkuskov/7902338/webrev.00/

Thank you,
Leonid

On 10/24/18 10:07, Jonathan Gibbons wrote:
> Leonid,
>
> Your use of Runtime.exec may cause problems.
>
> In particular, you call Process.waitFor before reading any output from 
> stdout, and not reading anything from stderr. This means that the 
> process may block if there is a lot of output from the command which 
> exceeds the available buffering.
>
> To avoid blocking, you should follow these rules:
>
> 1. Read from stdout and stderr in a timely manner, i.e. before calling 
> Process.waitFor
>
> 2. If you don't want to read from stdout and stderr separately, which 
> requires multiple threads, you should start the process with 
> ProcessBuilder, and set stderr to redirect to stdout using 
> ProcessBuilder.redirectErrorStream(boolean)
> https://download.java.net/java/early_access/jdk11/docs/api/java.base/java/lang/ProcessBuilder.html#redirectErrorStream(boolean) 
>
>
> 3. Read the streams to completion before calling Process.waitFor
>
> -- Jon
>
>
> On 10/23/18 3:22 PM, Leonid Kuskov wrote:
>> Thanks for the review, I've fixed  issues.
>>
>> Webrev: http://cr.openjdk.java.net/~lkuskov/7902337/webrev.01/
>>
>> Leonid
>>
>> On 10/23/18 13:58, Alexandre (Shura) Iline wrote:
>>> 1. Did you mean to use Objects.requireNonNull(Object) where you use 
>>> Objects.nonNull(Object)?
>>> 2. I think it makes sense to close the resources in 
>>> doCommand(String, File, String). Perhaps use try-with-resources.
>>> 3. In getJavaVersion(), the way I read it
>>> 3.1. VER9 will be returned for versions 16 or more, which is strange
>>> 3.2. For version 11, the return value will be 210 = 100 + 11*10
>>> 3.3. I would think just parsing the string with regex would give 
>>> more transparent code
>>>
>>> Shura
>>>
>>>> On Oct 23, 2018, at 1:20 PM, Leonid Kuskov 
>>>> <Leonid.Kuskov at Oracle.com> wrote:
>>>>
>>>> Please review and push the cosmetic fix for the
>>>>
>>>> Issue: https://bugs.openjdk.java.net/browse/CODETOOLS-7902337
>>>> Webrev: http://cr.openjdk.java.net/~lkuskov/7902337/webrev.00/
>>>>
>>>> Thanks,
>>>> Leonid
>>>>
>>
>