RFR: 8272600: (test) Use native "sleep" in Basic.java [v6]

David Holmes dholmes at openjdk.java.net
Tue Sep 21 05:01:49 UTC 2021


On Mon, 20 Sep 2021 13:01:29 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> The intermittent test in java/lang/ProcessBuilder/Basic.java has identified unexpected messages from a child Java VM
>> as the cause of the test failure.  Attempts to control the output of the child VM have failed, the VM is unrepentant .
>> 
>> There is no functionality in the child except to wait long enough for the test to finish and the child is destroyed.
>> The fix is to switch from using a Java child to using a native child; a new executable `sleepmillis`.
>
> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   The switch from a Java child to /bin/sleep caused another test
>   to fail on Linux.  The cleanup for a test used /usr/bin/pkill "sleep 60".
>   A race between that cleanup and subsequent tests that used sleep 60 or 600
>   could kill the sleep before the test of waitFor completed.
>   Changing the test using pkill to use 59 seconds makes the test cleanup
>   selective to the sleep spawned for that test.
>   The test that was failing (lines 2626-2624) passes consistently.

Hi Roger,

One suggestion based on your latest change, but that can be handled later. Otherwise this seems fine.

Thanks,
David

test/jdk/java/lang/ProcessBuilder/Basic.java line 2217:

> 2215:                 // A unique (59s) time is needed to avoid killing other sleep processes.
> 2216:                 final String[] cmd = { "/bin/bash", "-c", "(/bin/sleep 59)" };
> 2217:                 final String[] cmdkill = { "/bin/bash", "-c", "(/usr/bin/pkill -f \"sleep 59\")" };

Maybe future RFE but why do we even need pkill here when we can get the PID of the sleep process we create and kill only that process?

-------------

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5239


More information about the core-libs-dev mailing list