Fixed ICE-7947 - fixed whenSent semantics to not depend on sentSynchronously

9 files changed, 124 insertions, 207 deletions

diff --git a/java/src/Ice/src/main/java/com/zeroc/Ice/ConnectionI.java b/java/src/Ice/src/main/java/com/zeroc/Ice/ConnectionI.java
index 0aedf4dff08..db8a848147f 100644
--- a/java/src/Ice/src/main/java/com/zeroc/Ice/ConnectionI.java
+++ b/java/src/Ice/src/main/java/com/zeroc/Ice/ConnectionI.java
@@ -521,22 +521,9 @@ public final class ConnectionI extends com.zeroc.IceInternal.EventHandler
         }
 
         @Override
-        protected void markSent()
-        {
-            super.markSent();
-
-            assert((_state & StateOK) != 0);
-            complete(null);
-        }
-
-        @Override
         protected void markCompleted()
         {
-            if(_exception != null)
-            {
-                completeExceptionally(_exception);
-            }
-            super.markCompleted();
+            complete(null);
         }
 
         public void invoke()
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/CommunicatorFlushBatch.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/CommunicatorFlushBatch.java
index 3f1d39c3363..566d9dd5b70 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/CommunicatorFlushBatch.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/CommunicatorFlushBatch.java
@@ -28,11 +28,8 @@ public class CommunicatorFlushBatch extends InvocationFutureI<Void>
     }
 
     @Override
-    protected void markSent()
+    protected void markCompleted()
     {
-        super.markSent();
-
-        assert((_state & StateOK) != 0);
         complete(null);
     }
 
@@ -48,12 +45,6 @@ public class CommunicatorFlushBatch extends InvocationFutureI<Void>
             }
 
             @Override
-            protected void markSent()
-            {
-                assert(false);
-            }
-
-            @Override
             protected boolean needCallback()
             {
                 return false;
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectionFlushBatch.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectionFlushBatch.java
index 48e0b1a6f72..d9641d6a519 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectionFlushBatch.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ConnectionFlushBatch.java
@@ -27,22 +27,9 @@ public class ConnectionFlushBatch extends OutgoingAsyncBaseI<Void>
     }
 
     @Override
-    protected void markSent()
-    {
-        super.markSent();
-
-        assert((_state & StateOK) != 0);
-        complete(null);
-    }
-
-    @Override
     protected void markCompleted()
     {
-        if(_exception != null)
-        {
-            completeExceptionally(_exception);
-        }
-        super.markCompleted();
+        complete(null);
     }
 
     public void invoke(com.zeroc.Ice.CompressBatch compressBatch)
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 3be5400e401..0d444f50816 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
@@ -11,6 +11,7 @@ package com.zeroc.IceInternal;
 
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
+import java.util.function.BiConsumer;
 
 import com.zeroc.Ice.Communicator;
 import com.zeroc.Ice.CommunicatorDestroyedException;
@@ -113,8 +114,7 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
     }
 
     @Override
-    synchronized public final CompletableFuture<Boolean> whenSent(
-        java.util.function.BiConsumer<Boolean, ? super Throwable> action)
+    synchronized public final CompletableFuture<Boolean> whenSent(BiConsumer<Boolean, ? super Throwable> action)
     {
         if(_sentFuture == null)
         {
@@ -128,51 +128,27 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
         //
         if(((_state & StateSent) > 0 || _exception != null) && !_sentFuture.isDone())
         {
-            //
-            // The documented semantics state that a sent callback will be invoked from the
-            // calling thread if the request was sent synchronously. Calling complete() or
-            // completeExceptionally() on _sentFuture invokes the action from this thread.
-            //
-            if(_sentSynchronously)
+            if(_exception != null)
             {
-                if(_exception != null)
-                {
-                    _sentFuture.completeExceptionally(_exception);
-                }
-                else
-                {
-                    _sentFuture.complete(_sentSynchronously);
-                }
+                _sentFuture.completeExceptionally(_exception);
             }
             else
             {
-                if(_exception != null)
-                {
-                    dispatch(() ->
-                        {
-                            _sentFuture.completeExceptionally(_exception);
-                        });
-                }
-                else
-                {
-                    invokeSentAsync();
-                }
+                _sentFuture.complete(_sentSynchronously);
             }
         }
         return r;
     }
 
     @Override
-    synchronized public final CompletableFuture<Boolean> whenSentAsync(
-        java.util.function.BiConsumer<Boolean, ? super Throwable> action)
+    synchronized public final CompletableFuture<Boolean> whenSentAsync(BiConsumer<Boolean, ? super Throwable> action)
     {
         return whenSentAsync(action, null);
     }
 
     @Override
-    synchronized public final CompletableFuture<Boolean> whenSentAsync(
-        java.util.function.BiConsumer<Boolean, ? super Throwable> action,
-        Executor executor)
+    synchronized public final CompletableFuture<Boolean> whenSentAsync(BiConsumer<Boolean, ? super Throwable> action,
+                                                                       Executor executor)
     {
         if(_sentFuture == null)
         {
@@ -194,11 +170,6 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
         //
         if(((_state & StateSent) > 0 || _exception != null) && !_sentFuture.isDone())
         {
-            //
-            // When the caller uses whenSentAsync, we ignore the regular semantics and
-            // always complete the future from this thread. The caller's action will
-            // be invoked using the executor.
-            //
             if(_exception != null)
             {
                 _sentFuture.completeExceptionally(_exception);
@@ -211,26 +182,22 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
         return r;
     }
 
-    protected synchronized void markSent()
-    {
-        if(_sentFuture != null && !_sentFuture.isDone())
-        {
-            _sentFuture.complete(_sentSynchronously);
-        }
-    }
-
     public final void invokeSent()
     {
-        /* TBD
-        if(_instance.useApplicationClassLoader())
-        {
-            Thread.currentThread().setContextClassLoader(_callback.getClass().getClassLoader());
-        }
-        */
-
         try
         {
-            markSent();
+            synchronized(this)
+            {
+                if(_sentFuture != null && !_sentFuture.isDone())
+                {
+                    _sentFuture.complete(_sentSynchronously);
+                }
+            }
+
+            if(_doneInSent)
+            {
+                markCompleted();
+            }
         }
         catch(java.lang.RuntimeException ex)
         {
@@ -244,15 +211,6 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
                 throw exc;
             }
         }
-        finally
-        {
-            /* TBD
-            if(_instance.useApplicationClassLoader())
-            {
-                Thread.currentThread().setContextClassLoader(null);
-            }
-            */
-        }
 
         if(_observer != null)
         {
@@ -267,29 +225,30 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
 
     protected boolean needCallback()
     {
-        return true;
+        return !_synchronous; // No callbacks for synchronous or batch invocations
     }
 
-    protected void markCompleted()
-    {
-        if(_exception != null && _sentFuture != null)
-        {
-            _sentFuture.completeExceptionally(_exception);
-        }
-    }
+    abstract protected void markCompleted();
 
     public final void invokeCompleted()
     {
-        /* TBD
-        if(_instance.useApplicationClassLoader())
-        {
-            Thread.currentThread().setContextClassLoader(_callback.getClass().getClassLoader());
-        }
-        */
-
         try
         {
-            markCompleted();
+            if(_exception != null)
+            {
+                synchronized(this)
+                {
+                    if(_sentFuture != null && !_sentFuture.isDone())
+                    {
+                        _sentFuture.completeExceptionally(_exception);
+                    }
+                }
+                completeExceptionally(_exception);
+            }
+            else
+            {
+                markCompleted();
+            }
         }
         catch(RuntimeException ex)
         {
@@ -303,15 +262,6 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
         {
             error(exc);
         }
-        finally
-        {
-            /* TBD
-            if(_instance.useApplicationClassLoader())
-            {
-                Thread.currentThread().setContextClassLoader(null);
-            }
-            */
-        }
 
         if(_observer != null)
         {
@@ -359,6 +309,8 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
         _operation = op;
         _state = 0;
         _sentSynchronously = false;
+        _doneInSent = false;
+        _synchronous = false;
         _exception = null;
     }
 
@@ -378,6 +330,7 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
             {
                 _state |= StateDone | StateOK;
                 _cancellationHandler = null;
+                _doneInSent = true;
 
                 //
                 // For oneway requests after the data has been sent
@@ -388,8 +341,24 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
                 //
                 cacheMessageBuffers();
             }
-            this.notifyAll();
-            return !alreadySent;
+
+            boolean invoke = !alreadySent && (_sentFuture != null || done && needCallback());
+            if(!invoke && done && _observer != null)
+            {
+                _observer.detach();
+                _observer = null;
+            };
+
+            if(done && !needCallback())
+            {
+                markCompleted();
+                return false;
+            }
+            else
+            {
+                this.notifyAll();
+                return invoke;
+            }
         }
     }
 
@@ -403,16 +372,31 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
                 _state |= StateOK;
             }
             _cancellationHandler = null;
-            if(!needCallback())
+
+            boolean invoke = needCallback();
+            if(!invoke && _observer != null)
+            {
+                _observer.detach();
+                _observer = null;
+            }
+
+            if(!invoke)
             {
-                if(_observer != null)
+                if(_exception != null)
+                {
+                    completeExceptionally(_exception);
+                }
+                else
                 {
-                    _observer.detach();
-                    _observer = null;
+                    markCompleted();
                 }
+                return false;
+            }
+            else
+            {
+                this.notifyAll();
+                return invoke;
             }
-            this.notifyAll();
-            return needCallback();
         }
     }
 
@@ -427,16 +411,31 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
             {
                 _observer.failed(ex.ice_id());
             }
-            if(!needCallback())
+
+            boolean invoke = needCallback();
+            if(!invoke && _observer != null)
             {
-                if(_observer != null)
+                _observer.detach();
+                _observer = null;
+            }
+
+            if(!invoke)
+            {
+                if(_exception != null)
                 {
-                    _observer.detach();
-                    _observer = null;
+                    completeExceptionally(_exception);
                 }
+                else
+                {
+                    markCompleted();
+                }
+                return false;
+            }
+            else
+            {
+                this.notifyAll();
+                return invoke;
             }
-            this.notifyAll();
-            return needCallback();
         }
     }
 
@@ -508,6 +507,8 @@ public abstract class InvocationFutureI<T> extends com.zeroc.Ice.InvocationFutur
     protected com.zeroc.Ice.Instrumentation.InvocationObserver _observer;
     protected Connection _cachedConnection;
     protected boolean _sentSynchronously;
+    protected boolean _doneInSent;
+    protected boolean _synchronous; // True if this AMI request is being used for a generated synchronous invocation.
     protected CompletableFuture<Boolean> _sentFuture;
 
     protected final Communicator _communicator;
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/OutgoingAsync.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/OutgoingAsync.java
index a45f43a2e30..91fc3d3a6f9 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/OutgoingAsync.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/OutgoingAsync.java
@@ -160,20 +160,6 @@ public class OutgoingAsync<T> extends ProxyOutgoingAsyncBaseI<T>
     }
 
     @Override
-    protected void markSent()
-    {
-        super.markSent();
-
-        if(!_proxy.ice_isTwoway())
-        {
-            //
-            // For a non-twoway proxy, the invocation is completed after it is sent.
-            //
-            complete(null);
-        }
-    }
-
-    @Override
     public boolean sent()
     {
         return sent(!_proxy.ice_isTwoway()); // done = true if not a two-way proxy (no response expected)
@@ -218,13 +204,14 @@ public class OutgoingAsync<T> extends ProxyOutgoingAsyncBaseI<T>
     @Override
     protected void markCompleted()
     {
-        super.markCompleted();
-
         try
         {
-            if(_exception != null)
+            if(!_proxy.ice_isTwoway())
             {
-                completeExceptionally(_exception);
+                //
+                // For a non-twoway proxy, the invocation is completed after it is sent.
+                //
+                complete(null);
             }
             else if((_state & StateOK) > 0)
             {
@@ -383,7 +370,6 @@ public class OutgoingAsync<T> extends ProxyOutgoingAsyncBaseI<T>
     final private com.zeroc.Ice.EncodingVersion _encoding;
     private com.zeroc.Ice.InputStream _is;
 
-    private boolean _synchronous; // True if this AMI request is being used for a generated synchronous invocation.
     private Class<?>[] _userExceptions; // Valid user exceptions.
     private Unmarshaler<T> _unmarshal;
 }
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyFlushBatch.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyFlushBatch.java
index 9121ca7908e..a5bc5281797 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyFlushBatch.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyFlushBatch.java
@@ -20,35 +20,22 @@ public class ProxyFlushBatch extends ProxyOutgoingAsyncBaseI<Void>
     }
 
     @Override
-    public boolean completed(com.zeroc.Ice.InputStream is)
+    protected boolean needCallback()
     {
-        assert(false);
-        return false;
+        return !_synchronous;
     }
 
     @Override
-    protected synchronized void markSent()
-    {
-        super.markSent();
-
-        assert((_state & StateOK) != 0);
-        complete(null);
-    }
-
-    @Override
-    protected boolean needCallback()
+    public boolean completed(com.zeroc.Ice.InputStream is)
     {
-        return true;
+        assert(false);
+        return false;
     }
 
     @Override
     protected void markCompleted()
     {
-        super.markCompleted();
-        if(_exception != null)
-        {
-            completeExceptionally(_exception);
-        }
+        complete(null);
     }
 
     @Override
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyGetConnection.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyGetConnection.java
index 82dfad76ec2..22f9544fdea 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyGetConnection.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyGetConnection.java
@@ -20,22 +20,13 @@ public class ProxyGetConnection extends ProxyOutgoingAsyncBaseI<com.zeroc.Ice.Co
     @Override
     protected boolean needCallback()
     {
-        return true;
+        return !_synchronous;
     }
 
     @Override
     protected void markCompleted()
     {
-        super.markCompleted();
-
-        if(_exception != null)
-        {
-            completeExceptionally(_exception);
-        }
-        else
-        {
-            complete(_cachedConnection);
-        }
+        complete(_cachedConnection);
     }
 
     @Override
diff --git a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyIceInvoke.java b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyIceInvoke.java
index 4bfdbedddf9..6e17d0bbcaf 100644
--- a/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyIceInvoke.java
+++ b/java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyIceInvoke.java
@@ -102,20 +102,6 @@ public class ProxyIceInvoke extends ProxyOutgoingAsyncBaseI<com.zeroc.Ice.Object
     }
 
     @Override
-    protected void markSent()
-    {
-        super.markSent();
-
-        if(!_proxy.ice_isTwoway())
-        {
-            //
-            // For a non-twoway proxy, the invocation is completed after it is sent.
-            //
-            complete(new com.zeroc.Ice.Object.Ice_invokeResult(true, new byte[0]));
-        }
-    }
-
-    @Override
     public boolean sent()
     {
         return sent(!_proxy.ice_isTwoway()); // done = true if not a two-way proxy (no response expected)
@@ -160,11 +146,12 @@ public class ProxyIceInvoke extends ProxyOutgoingAsyncBaseI<com.zeroc.Ice.Object
     @Override
     protected void markCompleted()
     {
-        super.markCompleted();
-
-        if(_exception != null)
+        if(!_proxy.ice_isTwoway())
         {
-            completeExceptionally(_exception);
+            //
+            // For a non-twoway proxy, the invocation is completed after it is sent.
+            //
+            complete(new com.zeroc.Ice.Object.Ice_invokeResult(true, new byte[0]));
         }
         else
         {
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 5c49d464c6c..5adb9a3a86c 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
@@ -295,7 +295,7 @@ public abstract class ProxyOutgoingAsyncBaseI<T> extends OutgoingAsyncBaseI<T> i
     @Override
     protected boolean needCallback()
     {
-        return !isBatch();
+        return !_synchronous && !isBatch(); // No callbacks for synchronous or batch invocations
     }
 
     protected void invokeImpl(boolean userThread)