[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
jenkinsci · Dec 7, 2017 · 3f07563 · 3f07563
1 parent cdec8f8
commit 3f07563
Showing 1 changed file with 19 additions and 3 deletions.
diff --git a/src/main/java/hudson/remoting/AsyncFutureImpl.java b/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();
     }