On 2019-10-23 00:42, Magnus Ihse Bursie wrote:
On 2019-10-22 23:07, Erik Joelsson wrote:
When RunTest.gmk runs jtreg tests, it prints the exitcode of jtreg
into a file named exitcode.txt. Soemtimes, this fails and the
exitcode.txt file is left empty. This is causing trouble in automated
testing where the surrounding framework is expecting to check the
result in that file.
I believe this is caused by a race in bash. We have seen similar
races before and there is even a comment about it in the
documentation for the ExecuteWithLog macro. If there is redirect of
stdout in a command executed by ExecuteWithLog, that command must be
surrounded by parens to run in a subshell. I first discovered this
solution in JDK-8158629.
Yikes! It's not the first time, yes. Do you think it would make sense
to put in the extra parenthesis in the ExecuteWithLog itself? I guess
the main problem with this is that we incur an extra shell invocation
for each call, even if it's not needed. Normally this would not be
even noticeable, but on Windows it might..? Otoh, being race free is
more important, and leaving the responsibility for this to the caller
of ExecuteWithLog is just too easy to get wrong, as we have seen
multiple times.
Yes, I've been torn about this. On Windows I made measurable build speed
improvements some years ago by minimizing the extra shell invocations
for compile commands, so I didn't like the idea of introducing an
unconditional extra shell. Another option could be to scan the input for
'[<>]' in ExecuteWithLog and only add it if found. It will certainly
make the ExecuteWithLog body convoluted but may be a better solution.
This patch puts parens around the jtreg and other test framework
calls in RunTest.gmk.
Webrev: http://cr.openjdk.java.net/~erikj/8232834/webrev.jdk14.01/
Looks good to me. Even if we decide to change ExecuteWithLog itself
later on, I think it makes sense to fix this here and now.
Thanks!
/Erik
