RFR 9: 8064932: test java/lang/ProcessBuilder/Basic.java: waitFor didn't take long enough

roger riggs roger.riggs at oracle.com
Tue Nov 18 22:28:37 UTC 2014


Hi,

Done,  added the test and immediate return after the wait.
On Windows, the Thread.interrupted check needs to be done first to be 
consistent with Unix.
Refactored a bit to remove a local variable and extra arithmetic 
computing the remainingNanos.

Updated:
http://cr.openjdk.java.net/~rriggs/webrev-basic-notenough-8064932/

Roger



On 11/18/2014 4:58 PM, Martin Buchholz wrote:
> I think we're all 3 in agreement on the general direction, even if we
> disagree on the details.
>
> Once we properly round to millis, it is no longer necessary to use
> Math.max - just
>
> wait(NANOSECONDS.toMillis(rem + 999_999L));
>
> As in my proposed change, I would like every call to wait immediately
> followed by a check
> if (hasExited)
>    return true;
>
> to avoid the extra call to nanoTime.
>
> On Tue, Nov 18, 2014 at 12:09 PM, roger riggs <roger.riggs at oracle.com> wrote:
>> Hi,
>>
>> Work on Object.wait is an issue to be taken up separately.
>>
>> I agree that the timeout values should not be truncated to milliseconds,
>> likely
>> causing an additional cycle through the while loop and waiting.
>> The Process.waitFor methods are expected to wait for the specified time to
>> elapse.
>> The webrev has been updated for both Windows and Unix to use a ceiling
>> function
>> on milliseconds and ensure that at least the requested time has elapsed.
>>
>> Please review:
>>    http://cr.openjdk.java.net/~rriggs/webrev-basic-notenough-8064932/
>>
>> Thanks, Roger
>>
>>
>>
>>
>> On 11/18/2014 11:08 AM, Martin Buchholz wrote:
>>> On Mon, Nov 17, 2014 at 8:54 PM, David Holmes <david.holmes at oracle.com>
>>> wrote:
>>>> On 18/11/2014 2:43 PM, Martin Buchholz wrote:
>>>>> Proposed sibling change
>>>>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/UNIXProcess.waitFor/
>>>>> - don't unconditionally call nanoTime when the wait ends
>>>>> - use the millis/nanos form of Object.wait in case sub-millisecond
>>>>> waits are ever supported.
>>>>
>>>> I don't really see the point of adding the extra math, plus an extra call
>>>> (the two arg wait will call the one arg version) for no actual gain.
>>> The idea was to code to the Object.wait API, not its current
>>> implementation with only millisecond resolution.
>>> Even if we code to the current implementation, we should round UP not
>>> down,
>>> to avoid the problem of needing to call wait twice in case of early
>>> return.
>>> That would also be progress.
>>>
>>>> If you want to add a fast exit path that's fine but the rest seems
>>>> superfluous to me.
>>




More information about the core-libs-dev mailing list