Here is new version of the fix:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-7147084/webrev.03/
On 8/6/2013 8:48 AM, Alan Bateman wrote:
On 05/08/2013 09:05, Alexey Utkin wrote:
Here is new version of the fix:
http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-7147084/webrev.02/
Changes (in accordance with Alan's recommendations):
-- Magic constant -1 was changed to JAVA_INVALID_HANDLE_VALUE with
explanation why the macro
INVALID_HANDLE_VALUE is not in use.
-- The comments were added for each usage of the
JAVA_INVALID_HANDLE_VALUE macro.
-- 10 seconds time limit was rewritten to file signals.
Regards,
-uta
Thanks for adding comments. I still think initHolder will be confusing
to future maintainers and could benefit from a clear comment (above
the static BOOL ...) to explain the parameters. One typo in the
comment in " [thisProcessEnd] has no the inherit flag.". The other
functions are mostly clear now (thanks).
The sentence was changed, more comments were written.
There are a few formatting oddities in
Java_java_lang_ProcessImpl_create. Looks like a newline has crept into
the if statement at L337. Then L358-364 is mis-aligned which makes it
hard to tie the JNI ReleaseStringChars back to the JNI GetStringChars.
Thanks, that was [hg diff]/[patch] artifacts. Fixed.
Thanks for replacing the fragile 10s in the tests. If I read performC
(in both tests) correctly then the test will still pass after 5
minutes even if the JDK is broken - is that right? Just wonder if it
would be better to just replace the for loop with while (true) and let
the test harness timeout the test.
That is not so easy. The same program body is used 3 times in 3
different process: "A", "B", and "C". The process "A" life cycle in
managed by JPRT machinery. "B" is a short-living intermediate process.
"C" is a long-living "greedy" process. "C" process is unknown for JPRT
and is living till the signal from process "A", or 5 min if the test
failed. Hope that is long enough for slow machines.
I'm also wondering about waitAbit and whether this is useful.
[waitAbit] is used to force the thread context switching.
Otherwise I think this is good work and we are finally close to the
finish line.
-Alan.
