RFR: 8336479: Provide Process.waitFor(Duration) [v2]

[Naoto Sato]

On Thu, 18 Jul 2024 05:11:12 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Naoto Sato has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   ProcessTools overriding one-arg waitFor()
>
> src/java.base/share/classes/java/lang/Process.java line 501:
> 
>> 499:         if (hasExited())
>> 500:             return true;
>> 501:         if (duration.isZero() || duration.isNegative())
> 
> Hello Naoto, I see that there's a `Duration.isPositive()` API. Should we use that here instead?

Good point. I forgot it although I introduced the methd back in JDK18 🙂

> test/jdk/java/lang/Process/WaitForDuration.java line 57:
> 
>> 55:             throws IOException, InterruptedException {
>> 56:         assertEquals(expected,
>> 57:             new ProcessBuilder("sleep", "3").start().waitFor(d));
> 
> I think in its current form, this has a chance of failure (for inputs like 0 or negative duration), if the sleep (for 3 seconds) completes (and thus the process exits) before the `Process.waitFor` implementation has had a chance to execute `hasExited()`.
> 
> Also, this test is marked to run on all platforms. I think we might need special handling for `sleep` executable on Windows. In fact, looking at the `initSleepPath` in the `test/jdk/java/lang/ProcessBuilder/Basic.java` test, I suspect we might need something similar in this test even for *nix.

Made the sleep length variable, and used one hour for zero or negative (should be enough). For the positive, the process completes immediately. Also, I changed the sleep part to pure Java so that it won't rely on the testing platform.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20220#discussion_r1683292669
PR Review Comment: https://git.openjdk.org/jdk/pull/20220#discussion_r1683292765