On 25/02/2014 1:49 PM, Ivan Gerasimov wrote:
On 25.02.2014 6:52, David Holmes wrote:
On 25/02/2014 1:34 AM, Ivan Gerasimov wrote:
Thank you Roger for looking at the fix!
On 24.02.2014 18:37, roger riggs wrote:
Hi Ivan,
The code is correct as written but there might be some creep in the
end time due to the sampling of System.nanoTime.
I would be inclined to calculate the final time of the timeout once
and then compare simply with the current nanotime.
long end = (timeout == 0) ? Long.MAX_VALUE : (System.nanoTime() +
timeout * 1000000);
I don't think it is always correct.
According to the documentation, "The value returned represents
nanoseconds since some fixed but *arbitrary* origin time".
http://download.java.net/jdk8/docs/api/java/lang/System.html#nanoTime--
Thus, Long.MAX_VALUE may just happen to represent some valid time in the
near future, and we'll achieve wrong result.
Correct. Simplest thing is to calculate end ignoring whether timeout
is zero, and only check "now < end" if timeout != 0.
David, as Martin said before, 'now < end' may hold true even if now is
after end (now is negative).
In my version of the code, I operate on the difference (after - before),
thus avoiding the possible overflow.
Sorry, I put "now < end" in quotes to mean the effect of now<end, the
actual calculation should still be a subtraction as Martin indicated.
However I was overlooking the fact you need to reduce the timeout for
subsequent waits.
David
Sincerely yours,
Ivan
David
Sincerely yours,
Ivan
Then the test in the loop can be:
if (System.nanoTime() > end) {
return null;
}
Roger (Not a Reviewer)
On 2/24/2014 12:59 AM, Ivan Gerasimov wrote:
Hello!
ReferenceQueue.remove(timeout) may return too early, i.e. before the
specified timeout has elapsed.
Would you please review the fix?
The change also includes a regression test, which can be used to
demonstrate the issue.
BUGURL: https://bugs.openjdk.java.net/browse/JDK-6853696
WEBREV: http://cr.openjdk.java.net/~igerasim/6853696/0/webrev/
Sincerely yours,
Ivan
