http://cr.openjdk.java.net/~almatvee/8235738/webrev.01/
Fixed testForPresenceOnly spelling error.
Thanks,
Alexander
On 12/13/2019 9:39 AM, Phil Race wrote:
testForPresenseOnly It should be spelt testForPresenceOnly -phil.
On 12/13/19 6:16 AM, Andy Herrick wrote:
I approve these changes.
My first thought was that, if reading output only after Process is
complete is valid and safe, then why not do it that way all the time
? But comment in Process javadoc: "Because some native platforms
only provide limited buffer size for standard input and output
streams, failure to promptly write the input stream or read the
output stream of the process may cause the process to block, or even
deadlock." indicates is is prudent to do this only when necessary.
My second thought is that this indicates an underlying unidentified
bug in the mac native code used by Process or ProcessBuilder , and
although it would be better to identify and resolve the underlying
problem, the fix is needed now, so this change seems appropriate.
/Andy
On 12/13/2019 1:01 AM, Alexander Matveev wrote:
Please review fix [2] for jpackage bug [1].
Not sure why it happens, but reading output from "hdiutil attach"
was not exiting immediately after process terminated with delays
upto 20 seconds always and in some case upto 10 minutes and thus
test was timeout. Only possible workaround found is to wait for
"hdiutil attach" process to exit before reading output. In this case
call to "hdiutil attach" and reading output happens very fast and
does not hang for long time. Generating simple DMG image went from
~40 seconds to ~20 seconds after fix. Test was done with verbose
output enabled.
Thanks,
Alexander
[1] https://bugs.openjdk.java.net/browse/JDK-8235738
[2] http://cr.openjdk.java.net/~almatvee/8235738/webrev.00/
