]
Bela Ban commented on JGRP-1877:
--------------------------------
Hmm... that would set the interrupt flag coming out of this method even if only 1 of 10
iterations ended up in the catch clause. Not sure what the right semantics should be, e.g.
if a cond.wait() terminated correctly, should {{intr}} be set back to false ?
System.nanoTime() can be negative
---------------------------------
Key: JGRP-1877
URL:
https://issues.jboss.org/browse/JGRP-1877
Project: JGroups
Issue Type: Bug
Reporter: Bela Ban
Assignee: Bela Ban
Fix For: 3.5.1, 3.6
According to the javadoc, {{System.nanoTime()}} should only be used to measure _elapsed
time_, but not compute a _target time in the future_, as {{nanoTime()}} might return a a
time in the future.
Code like the one below might fail:
{code:title=Responses.waitFor()|borderStyle=solid}
public boolean waitFor(long timeout) {
long wait_time;
final long target_time=System.nanoTime() + TimeUnit.NANOSECONDS.convert(timeout,
TimeUnit.MILLISECONDS); // ns
lock.lock();
try {
while(!done && (wait_time=target_time - System.nanoTime()) > 0) {
try {
cond.await(wait_time,TimeUnit.NANOSECONDS);
}
catch(InterruptedException e) {
}
}
return done;
}
finally {
lock.unlock();
}
}
{code}
When computing {{target_time}}, {{System.nanoTime()}} could return a negative value
(numeric overflow) or a value in the future. In the first case, {{target_time}} could be
negative, so the method would not block at all. In the latter case, {{target_time}} could
be huge, so the method would block for a long time.
Investigate all occurrences where we use {{nanoTime()}} to compute a time in the future,
and see what impact a future value value could have. Possibly replace with
{{System.currentTimeMillis()}} or the _time service_.