Fix bug in calling netty buffer release for Async GetWorkflowExecutionHistory (#493)

Author: meiliang86

src/main/java/com/uber/cadence/client/WorkflowOptions.java

@@ -124,9 +124,10 @@ public Builder setWorkflowId(String workflowId) {
      * conditions Cadence allows two workflows with the same domain and workflow id run
      * simultaneously.
      * <li>
-     * <ul>
-     *   AllowDuplicateFailedOnly is a default value. It means that workflow can start if
-     *   previous run failed or was cancelled or terminated.
+     *
+     *     <ul>
+     *       AllowDuplicateFailedOnly is a default value. It means that workflow can start if
+     *       previous run failed or was cancelled or terminated.
      * </ul>
      *
      * <ul>
@@ -139,9 +140,8 @@ public Builder setWorkflowId(String workflowId) {
      *
      * <ul>
      *   TerminateIfRunning terminate current running workflow using the same workflow ID if exist,
-     * 	 then start a new run in one transaction
+     *   then start a new run in one transaction
      * </ul>
-     * </li>
      */
     public Builder setWorkflowIdReusePolicy(WorkflowIdReusePolicy workflowIdReusePolicy) {
       this.workflowIdReusePolicy = workflowIdReusePolicy;

src/main/java/com/uber/cadence/serviceclient/WorkflowServiceTChannel.java

@@ -2302,14 +2302,14 @@ public void StartWorkflowExecution(
   public void GetWorkflowExecutionHistory(
       GetWorkflowExecutionHistoryRequest getRequest, AsyncMethodCallback resultHandler) {
     CompletableFuture<ThriftResponse<GetWorkflowExecutionHistory_result>> response = null;
-    try {
-      ThriftRequest<WorkflowService.GetWorkflowExecutionHistory_args> request =
-          buildGetWorkflowExecutionHistoryThriftRequest(getRequest);
-      response = doRemoteCallAsync(request);
-
-      response
-          .whenComplete(
-              (r, e) -> {
+    ThriftRequest<WorkflowService.GetWorkflowExecutionHistory_args> request =
+        buildGetWorkflowExecutionHistoryThriftRequest(getRequest);
+    response = doRemoteCallAsync(request);
+
+    response
+        .whenComplete(
+            (r, e) -> {
+              try {
                 if (e != null) {
                   resultHandler.onError(CheckedExceptionWrapper.wrap(e));
                   return;
@@ -2336,18 +2336,17 @@ public void GetWorkflowExecutionHistory(
                 resultHandler.onError(
                     new TException(
                         "GetWorkflowExecutionHistory failed with unknown " + "error:" + result));
-                return;
-              })
-          .exceptionally(
-              (e) -> {
-                log.error("Unexpected error in GetWorkflowExecutionHistory", e);
-                return null;
-              });
-    } finally {
-      if (response != null && response.isDone()) {
-        response.join().release();
-      }
-    }
+              } finally {
+                if (r != null) {
+                  r.release();
+                }
+              }
+            })
+        .exceptionally(
+            (e) -> {
+              log.error("Unexpected error in GetWorkflowExecutionHistory", e);
+              return null;
+            });
   }
 
   @Override

src/test/java/com/uber/cadence/workflow/WorkflowTest.java

@@ -18,7 +18,13 @@
 package com.uber.cadence.workflow;
 
 import static com.uber.cadence.worker.NonDeterministicWorkflowPolicy.FailWorkflow;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import com.google.common.util.concurrent.UncheckedExecutionException;
 import com.uber.cadence.GetWorkflowExecutionHistoryResponse;