Fixed retry bug with -2 invocation timeout, fixes #501

5 files changed, 73 insertions, 30 deletions

diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/InvocationFutureI.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/InvocationFutureI.java
index e070990e93c..bf0306dd809 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/InvocationFutureI.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/InvocationFutureI.java
@@ -444,9 +444,9 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
         CancellationHandler handler;
         synchronized(this)
         {
-            _cancellationException = ex;
             if(_cancellationHandler == null)
             {
+                _cancellationException = ex;
                 return;
             }
             handler = _cancellationHandler;
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyOutgoingAsyncBaseI.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyOutgoingAsyncBaseI.java
index 9ef6a1a601b..86cdaf90cf1 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyOutgoingAsyncBaseI.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyOutgoingAsyncBaseI.java
@@ -184,6 +184,13 @@ public abstract class ProxyOutgoingAsyncBaseI<T> extends OutgoingAsyncBaseI<T> i
             _childObserver = null;
         }
 
+        _cachedConnection = null;
+        if(_proxy._getReference().getInvocationTimeout() == -2 && _timerFuture != null)
+        {
+            _timerFuture.cancel(false);
+            _timerFuture = null;
+        }
+
         //
         // NOTE: at this point, synchronization isn't needed, no other threads should be
         // calling on the callback.
diff --git a/java/test/src/main/java/test/Ice/retry/AllTests.java b/java/test/src/main/java/test/Ice/retry/AllTests.java
index a914159f5f9..4548297f550 100644
--- a/java/test/src/main/java/test/Ice/retry/AllTests.java
+++ b/java/test/src/main/java/test/Ice/retry/AllTests.java
@@ -218,37 +218,55 @@ public class AllTests
             out.println("ok");
         }
 
-        out.print("testing invocation timeout and retries... ");
-        out.flush();
-
-        retry2 = RetryPrx.checkedCast(communicator2.stringToProxy(retry1.toString()));
-        try
-        {
-            // No more than 2 retries before timeout kicks-in
-            retry2.ice_invocationTimeout(500).opIdempotent(4);
-            test(false);
-        }
-        catch(com.zeroc.Ice.InvocationTimeoutException ex)
-        {
-            instrumentation.testRetryCount(2);
-            retry2.opIdempotent(-1); // Reset the counter
-            instrumentation.testRetryCount(-1);
-        }
-        try
         {
-            // No more than 2 retries before timeout kicks-in
-            RetryPrx prx = retry2.ice_invocationTimeout(500);
-            prx.opIdempotentAsync(4).join();
-            test(false);
-        }
-        catch(java.util.concurrent.CompletionException ex)
-        {
-            test(ex.getCause() instanceof com.zeroc.Ice.InvocationTimeoutException);
-            instrumentation.testRetryCount(2);
-            retry2.opIdempotent(-1); // Reset the counter
-            instrumentation.testRetryCount(-1);
+            out.print("testing invocation timeout and retries... ");
+            out.flush();
+
+            retry2 = RetryPrx.checkedCast(communicator2.stringToProxy(retry1.toString()));
+            try
+            {
+                // No more than 2 retries before timeout kicks-in
+                retry2.ice_invocationTimeout(500).opIdempotent(4);
+                test(false);
+            }
+            catch(com.zeroc.Ice.InvocationTimeoutException ex)
+            {
+                instrumentation.testRetryCount(2);
+                retry2.opIdempotent(-1); // Reset the counter
+                instrumentation.testRetryCount(-1);
+            }
+            try
+            {
+                // No more than 2 retries before timeout kicks-in
+                RetryPrx prx = retry2.ice_invocationTimeout(500);
+                prx.opIdempotentAsync(4).join();
+                test(false);
+            }
+            catch(java.util.concurrent.CompletionException ex)
+            {
+                test(ex.getCause() instanceof com.zeroc.Ice.InvocationTimeoutException);
+                instrumentation.testRetryCount(2);
+                retry2.opIdempotent(-1); // Reset the counter
+                instrumentation.testRetryCount(-1);
+            }
+            if(retry1.ice_getConnection() != null)
+            {
+                // The timeout might occur on connection establishment or because of the sleep. What's
+                // important here is to make sure there are 4 retries and that no calls succeed to
+                // ensure retries with the old connection timeout semantics work.
+                RetryPrx retryWithTimeout = (RetryPrx)retry1.ice_invocationTimeout(-2).ice_timeout(200);
+                try
+                {
+                    retryWithTimeout.sleep(300);
+                    test(false);
+                }
+                catch(com.zeroc.Ice.ConnectionTimeoutException ex)
+                {
+                }
+                instrumentation.testRetryCount(4);
+            }
+            out.println("ok");
         }
-        out.println("ok");
 
         return retry1;
     }
diff --git a/java/test/src/main/java/test/Ice/retry/RetryI.java b/java/test/src/main/java/test/Ice/retry/RetryI.java
index 1ca6f95e0d0..0d62653173a 100644
--- a/java/test/src/main/java/test/Ice/retry/RetryI.java
+++ b/java/test/src/main/java/test/Ice/retry/RetryI.java
@@ -61,6 +61,22 @@ public final class RetryI implements Retry
     }
 
     @Override
+    public void sleep(int delay, com.zeroc.Ice.Current c)
+    {
+        while(true)
+        {
+            try
+            {
+                Thread.sleep(delay);
+                break;
+            }
+            catch(InterruptedException ex)
+            {
+            }
+        }
+    }
+
+    @Override
     public void shutdown(com.zeroc.Ice.Current current)
     {
         current.adapter.getCommunicator().shutdown();
diff --git a/java/test/src/main/java/test/Ice/retry/Test.ice b/java/test/src/main/java/test/Ice/retry/Test.ice
index 173ded096d7..8df6a93e8cf 100644
--- a/java/test/src/main/java/test/Ice/retry/Test.ice
+++ b/java/test/src/main/java/test/Ice/retry/Test.ice
@@ -16,6 +16,8 @@ interface Retry
     void opNotIdempotent();
     void opSystemException();
 
+    idempotent void sleep(int delay);
+
     idempotent void shutdown();
 }