Hi, Roger
On Windows, the test did not check liveness, but will check it now;
seems desirable.
I think the changes look fine as they are. Additional refactoring
possibilities for your consideration, to take or leave:
* ProcessTest::isAlive() is not used
* killProc() no longer needs a boolean argument
* the killProc() code could be moved into runTest()
-Brent
On 1/14/19 8:56 AM, Roger Riggs wrote:
Please review removing a test for Process.destroy(). [1]
It fails intermittently and is based on an incorrect assumption.
The child is a bash script that uses trap to ignore SIGTERM. The child
is started and then sent SIGTERM.
The child should not terminate. However, there is a race in which in
some cases the child does terminate
with SIGPIPE (not SIGTERM) as a result of destroy() closing the streams.
The Process implementation on Unix closes the streams after sending the
SIGTERM signal
and has since (forever...). But this behavior is not documented.
This test of destroy() is invalid and should be removed. Since both Mac
OS and Windows
already skip the testing of destroy() the test is simplified to remove
it from all cases.
A separate issue[2] has been created to consider documenting the
Process implementations' closing of the streams.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-destroytest-8080569-2/
Thanks, Roger
[1] https://bugs.openjdk.java.net/browse/JDK-8080569
[2] https://bugs.openjdk.java.net/browse/JDK-8216990
