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
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
