Skip to content
Permalink
Browse files

[JENKINS-48309] - Prevent TimeoutException in AsyncFutureImpl in the …

…case of spurious wakeups (#240)

* [JENKINS-48309] - Prevent TimeoutException in AsyncFutureImpl in the case of spurious wakeups.

TL;DR: A large part of Jenkins’ get-with-timeout logic is a subject for improper exit before the timeout.

* [JENKINS-48309] - I was too lazy to use nanos in the beginning, address the comment from @stephenc

* [JENKINS-48309] - Use wait with nanos in hope there are Java implementations which really support nano timeouts.

* [JENKINS-48309] - And again, @stephenc prevents a major embarassement due to the stupid typo
  • Loading branch information...
oleg-nenashev committed Dec 7, 2017
1 parent cdec8f8 commit 3f07563743ef1874057eeb17e04847170436b39f
Showing with 19 additions and 3 deletions.
  1. +19 −3 src/main/java/hudson/remoting/AsyncFutureImpl.java
@@ -23,6 +23,8 @@
*/
package hudson.remoting;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnegative;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
@@ -78,13 +80,27 @@ public synchronized V get() throws InterruptedException, ExecutionException {
return value;
}

public synchronized V get(long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException {
if(!completed)
wait(unit.toMillis(timeout));
@CheckForNull
public synchronized V get(@Nonnegative long timeout, TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException {
// The accuracy of wait(long) operation is milliseconds anyway, but ok.
long endWaitTime = System.nanoTime() + unit.toNanos(timeout);
while (!completed) {
long timeToWait = endWaitTime - System.nanoTime();
if (timeToWait < 0) {
break;
}

wait(timeToWait / 1000000, (int)(timeToWait % 1000000));
}

if(!completed)
throw new TimeoutException();
if(cancelled)
throw new CancellationException();

//TODO: we may be calling a complex get() implementation in the override, which may hang the code
// The only missing behavior is "problem!=null" branch, but probably we cannot just replace the code without
// an override issue risk.
return get();
}

0 comments on commit 3f07563

Please sign in to comment.
You can’t perform that action at this time.