[dubbo] branch 2.7.3-release updated: check complete status of async-result before try to update (#4394)

[victory]

This is an automated email from the ASF dual-hosted git repository.

victory pushed a commit to branch 2.7.3-release
in repository https://gitbox.apache.org/repos/asf/dubbo.git


The following commit(s) were added to refs/heads/2.7.3-release by this push:
     new 69a0483  check complete status of async-result before try to update 
(#4394)
69a0483 is described below

commit 69a0483c949b3d9ea384264b9f89539674cdef13
Author: ken.lj <ken.lj...@gmail.com>
AuthorDate: Mon Jul 1 10:54:11 2019 +0800

    check complete status of async-result before try to update (#4394)
    
    #4394
---
 .../java/org/apache/dubbo/rpc/AsyncRpcResult.java  | 40 ++++++++++++++++++----
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git 
a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AsyncRpcResult.java
 
b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AsyncRpcResult.java
index 343cf03..2714409 100644
--- 
a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AsyncRpcResult.java
+++ 
b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AsyncRpcResult.java
@@ -72,11 +72,30 @@ public class AsyncRpcResult extends AbstractResult {
         return getAppResponse().getValue();
     }
 
+    /**
+     * CompletableFuture can only be completed once, so try to update the 
result of one completed CompletableFuture will
+     * has no effect. To avoid this problem, we check the complete status of 
this future before update it's value.
+     *
+     * But notice that trying to give an uncompleted CompletableFuture a new 
specified value may face a race condition,
+     * because the background thread watching the real result will also change 
the status of this CompletableFuture.
+     * The result is you may lose the value you expected to set.
+     *
+     * @param value
+     */
     @Override
     public void setValue(Object value) {
-        AppResponse appResponse = new AppResponse();
-        appResponse.setValue(value);
-        this.complete(appResponse);
+        try {
+            if (this.isDone()) {
+                this.get().setValue(value);
+            } else {
+                AppResponse appResponse = new AppResponse();
+                appResponse.setValue(value);
+                this.complete(appResponse);
+            }
+        } catch (Exception e) {
+            // This should never happen;
+            logger.error("Got exception when trying to change the value of the 
underlying result from AsyncRpcResult.", e);
+        }
     }
 
     @Override
@@ -86,9 +105,18 @@ public class AsyncRpcResult extends AbstractResult {
 
     @Override
     public void setException(Throwable t) {
-        AppResponse appResponse = new AppResponse();
-        appResponse.setException(t);
-        this.complete(appResponse);
+        try {
+            if (this.isDone()) {
+                this.get().setException(t);
+            } else {
+                AppResponse appResponse = new AppResponse();
+                appResponse.setException(t);
+                this.complete(appResponse);
+            }
+        } catch (Exception e) {
+            // This should never happen;
+            logger.error("Got exception when trying to change the value of the 
underlying result from AsyncRpcResult.", e);
+        }
     }
 
     @Override