I was staring at that old process code I wrote many years ago, and I think it can be improved. I'll post a patch later.
On Mon, Nov 17, 2014 at 2:02 PM, roger riggs <roger.ri...@oracle.com> wrote: > Hi, > > The technique used in the Linux version of Process.waitFor() is applied to > the Windows version. The duration of the native wait is measured > using nanoTime and the wait is repeated as necessary. > For most uses, some jitter is expected due to workload, clock resolution, > etc. > > Webrev: > http://cr.openjdk.java.net/~rriggs/webrev-basic-notenough-8064932/ > > Thanks, Roger > > > > > On 11/17/2014 2:54 PM, Martin Buchholz wrote: >> >> Returning early is EVIL. >> ("""What part of 'wait for NNN nanoseconds' did you not understand??""") >> Unfortunately, Object.wait may do so. And perhaps also >> waitForMultipleObjects. >> HIgher level libraries need to be paranoid and compensate. >> >> On Mon, Nov 17, 2014 at 11:27 AM, roger riggs <roger.ri...@oracle.com> >> wrote: >>> >>> Hi, >>> >>> I need to go back and identify the platform of the failure; it is >>> failing >>> on Windows >>> so the correct code is in ProcessImpl_md.c in waitForTimeoutInterruptibly >>> in which the timeout is in milliseconds. >>> >>> If waitForMultipleObjects can return 'early' then the same kind of loop >>> used on Linux for testing nanoTime may be needed. >>> >>> Roger >>> >>> >>> >>> >>> On 11/14/2014 7:38 PM, Martin Buchholz wrote: >>>> >>>> Hi Roger, >>>> >>>> I keep staring at the code in UNIXProcess.java and am having trouble >>>> imagining how waitFor could possibly return early - that loop can only >>>> terminate when elapsed time as measured by System.nanoTime exceeds >>>> timeoutAsNanos. It's true that we might have truncation when >>>> converting to millis, but then the wait should get called a second >>>> time with arg 1 (which is suboptimal, yes - we should improve waitFor >>>> (and very sadly, we should also improve Object.wait)). >>>> >>>> Is there a way for me to repro this test failure? The JDK tries hard >>>> to provide monotonic nanoTime(). >>>> >>>> On Fri, Nov 14, 2014 at 2:15 PM, roger riggs <roger.ri...@oracle.com> >>>> wrote: >>>>> >>>>> Hi Martin, >>>>> >>>>> The artifact is by-product of using System.nanoTime to measure the >>>>> duration >>>>> in UNIXProcess and the conversion to milliseconds to call >>>>> Object.wait(): >>>>> >>>>> The low order bits of nanoseconds are truncated in the conversion. >>>>> >>>>> long timeoutAsNanos = unit.toNanos(timeout); >>>>> long startTime = System.nanoTime(); >>>>> long rem = timeoutAsNanos; >>>>> >>>>> while (!hasExited && (rem > 0)) { >>>>> wait(Math.max(TimeUnit.NANOSECONDS.toMillis(rem), 1)); >>>>> rem = timeoutAsNanos - (System.nanoTime() - startTime); >>>>> } >>>>> >>>>> The issue may be further complicated by the test logic also doing the >>>>> timing in nanoSeconds when the best Object.wait can do is milliseconds. >>>>> >>>>> If System.nanoTime is to be used, perhaps it should be modulo >>>>> milliseconds. >>>>> >>>>> Roger >>>>> >>>>> >>>>> >>>>> On 11/14/2014 2:57 PM, Martin Buchholz wrote: >>>>>> >>>>>> This sort of change may be a necessary concession to reality, but >>>>>> before we go there ... as I said in a previous review, this test may >>>>>> demonstrate a real bug in the jdk implementation. Is this >>>>>> system-dependent? Is it easy to reproduce? Can we fix the JDK? >>>>>> >>>>>> On Fri, Nov 14, 2014 at 11:48 AM, roger riggs <roger.ri...@oracle.com> >>>>>> wrote: >>>>>>> >>>>>>> Please review this test change to make the wait time in >>>>>>> ProcessBuilder/Basic >>>>>>> a bit more lenient. >>>>>>> >>>>>>> Webrev: >>>>>>> http://cr.openjdk.java.net/~rriggs/webrev-basic-notenough-8064932/ >>>>>>> >>>>>>> Issue: >>>>>>> 8064932: java/lang/ProcessBuilder/Basic.java: waitFor didn't >>>>>>> take >>>>>>> long >>>>>>> enough >>>>>>> >>>>>>> Thanks, Roger >>>>>>> >
