1 files changed, 114 insertions, 23 deletions

diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c
index dc3acc89bb..4054723b43 100644
--- a/src/php/ext/grpc/channel.c
+++ b/src/php/ext/grpc/channel.c
@@ -41,6 +41,7 @@
 
 #include <grpc/grpc.h>
 #include <grpc/grpc_security.h>
+#include <grpc/support/alloc.h>
 
 #include "completion_queue.h"
 #include "channel_credentials.h"
@@ -56,22 +57,63 @@ int le_plink;
 
 /* Frees and destroys an instance of wrapped_grpc_channel */
 PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel)
+  bool is_last_wrapper = false;
+  // In_persistent_list is used when the user don't close the channel.
+  // In this case, le in the list won't be freed.
+  bool in_persistent_list = true;
   if (p->wrapper != NULL) {
     gpr_mu_lock(&p->wrapper->mu);
     if (p->wrapper->wrapped != NULL) {
-      php_grpc_zend_resource *rsrc;
-      php_grpc_int key_len = strlen(p->wrapper->key);
-      // only destroy the channel here if not found in the persistent list
-      gpr_mu_lock(&global_persistent_list_mu);
-      if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&EG(persistent_list), p->wrapper->key,
-                                          key_len, rsrc))) {
-        grpc_channel_destroy(p->wrapper->wrapped);
-        free(p->wrapper->target);
-        free(p->wrapper->args_hashstr);
+      if (p->wrapper->is_valid) {
+        php_grpc_zend_resource *rsrc;
+        php_grpc_int key_len = strlen(p->wrapper->key);
+        // only destroy the channel here if not found in the persistent list
+        gpr_mu_lock(&global_persistent_list_mu);
+        if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&EG(persistent_list), p->wrapper->key,
+                                            key_len, rsrc))) {
+          in_persistent_list = false;
+          grpc_channel_destroy(p->wrapper->wrapped);
+          free(p->wrapper->target);
+          free(p->wrapper->args_hashstr);
+          if(p->wrapper->creds_hashstr != NULL){
+            free(p->wrapper->creds_hashstr);
+            p->wrapper->creds_hashstr = NULL;
+          }
+        }
+        gpr_mu_unlock(&global_persistent_list_mu);
       }
-      gpr_mu_unlock(&global_persistent_list_mu);
+    }
+    p->wrapper->ref_count -= 1;
+    if (p->wrapper->ref_count == 0) {
+      is_last_wrapper = true;
     }
     gpr_mu_unlock(&p->wrapper->mu);
+    if (is_last_wrapper) {
+      if (in_persistent_list) {
+        // If ref_count==0 and the key still in the list, it means the user
+        // don't call channel->close().persistent list should free the
+        // allocation in such case, as well as related wrapped channel.
+        if (p->wrapper->wrapped != NULL) {
+          gpr_mu_lock(&p->wrapper->mu);
+          grpc_channel_destroy(p->wrapper->wrapped);
+          free(p->wrapper->target);
+          free(p->wrapper->args_hashstr);
+          if(p->wrapper->creds_hashstr != NULL){
+            free(p->wrapper->creds_hashstr);
+            p->wrapper->creds_hashstr = NULL;
+          }
+          p->wrapper->wrapped = NULL;
+          php_grpc_delete_persistent_list_entry(p->wrapper->key,
+                                                strlen(p->wrapper->key)
+                                                TSRMLS_CC);
+          gpr_mu_unlock(&p->wrapper->mu);
+        }
+      }
+      gpr_mu_destroy(&p->wrapper->mu);
+      free(p->wrapper->key);
+      free(p->wrapper);
+    }
+    p->wrapper = NULL;
   }
 PHP_GRPC_FREE_WRAPPED_FUNC_END()
 
@@ -242,6 +284,7 @@ PHP_METHOD(Channel, __construct) {
 
   // parse the rest of the channel args array
   if (php_grpc_read_args_array(args_array, &args TSRMLS_CC) == FAILURE) {
+    efree(args.args);
     return;
   }
 
@@ -275,9 +318,16 @@ PHP_METHOD(Channel, __construct) {
   channel->wrapper->key = key;
   channel->wrapper->target = strdup(target);
   channel->wrapper->args_hashstr = strdup(sha1str);
+  channel->wrapper->creds_hashstr = NULL;
+  channel->wrapper->ref_count = 1;
+  channel->wrapper->is_valid = true;
   if (creds != NULL && creds->hashstr != NULL) {
-    channel->wrapper->creds_hashstr = creds->hashstr;
+    php_grpc_int creds_hashstr_len = strlen(creds->hashstr);
+    char *channel_creds_hashstr = malloc(creds_hashstr_len + 1);
+    strcpy(channel_creds_hashstr, creds->hashstr);
+    channel->wrapper->creds_hashstr = channel_creds_hashstr;
   }
+
   gpr_mu_init(&channel->wrapper->mu);
   smart_str_free(&buf);
 
@@ -301,7 +351,18 @@ PHP_METHOD(Channel, __construct) {
       create_and_add_channel_to_persistent_list(
           channel, target, args, creds, key, key_len TSRMLS_CC);
     } else {
+      efree(args.args);
+      if (channel->wrapper->creds_hashstr != NULL){
+        free(channel->wrapper->creds_hashstr);
+        channel->wrapper->creds_hashstr = NULL;
+      }
+      free(channel->wrapper->creds_hashstr);
+      free(channel->wrapper->key);
+      free(channel->wrapper->target);
+      free(channel->wrapper->args_hashstr);
+      free(channel->wrapper);
       channel->wrapper = le->channel;
+      channel->wrapper->ref_count += 1;
     }
   }
 }
@@ -321,7 +382,8 @@ PHP_METHOD(Channel, getTarget) {
   }
   char *target = grpc_channel_get_target(channel->wrapper->wrapped);
   gpr_mu_unlock(&channel->wrapper->mu);
-  PHP_GRPC_RETURN_STRING(target, 1);
+  PHP_GRPC_RETVAL_STRING(target, 1);
+  gpr_free(target);
 }
 
 /**
@@ -409,18 +471,46 @@ PHP_METHOD(Channel, watchConnectivityState) {
  */
 PHP_METHOD(Channel, close) {
   wrapped_grpc_channel *channel = Z_WRAPPED_GRPC_CHANNEL_P(getThis());
-  gpr_mu_lock(&channel->wrapper->mu);
-  if (channel->wrapper->wrapped != NULL) {
-    grpc_channel_destroy(channel->wrapper->wrapped);
-    free(channel->wrapper->target);
-    free(channel->wrapper->args_hashstr);
-    channel->wrapper->wrapped = NULL;
-
-    php_grpc_delete_persistent_list_entry(channel->wrapper->key,
-                                          strlen(channel->wrapper->key)
-                                          TSRMLS_CC);
+  bool is_last_wrapper = false;
+  if (channel->wrapper != NULL) {
+    // Channel_wrapper hasn't call close before.
+    gpr_mu_lock(&channel->wrapper->mu);
+    if (channel->wrapper->wrapped != NULL) {
+      if (channel->wrapper->is_valid) {
+        // Wrapped channel hasn't been destoryed by other wrapper.
+        grpc_channel_destroy(channel->wrapper->wrapped);
+        free(channel->wrapper->target);
+        free(channel->wrapper->args_hashstr);
+        if(channel->wrapper->creds_hashstr != NULL){
+          free(channel->wrapper->creds_hashstr);
+          channel->wrapper->creds_hashstr = NULL;
+        }
+        channel->wrapper->wrapped = NULL;
+        channel->wrapper->is_valid = false;
+
+        php_grpc_delete_persistent_list_entry(channel->wrapper->key,
+                                              strlen(channel->wrapper->key)
+                                              TSRMLS_CC);
+      }
+    }
+    channel->wrapper->ref_count -= 1;
+    if(channel->wrapper->ref_count == 0){
+      // Mark that the wrapper can be freed because mu should be
+      // destroyed outside the lock.
+      is_last_wrapper = true;
+    }
+    gpr_mu_unlock(&channel->wrapper->mu);
   }
-  gpr_mu_unlock(&channel->wrapper->mu);
+  gpr_mu_lock(&global_persistent_list_mu);
+  if (is_last_wrapper) {
+    gpr_mu_destroy(&channel->wrapper->mu);
+    free(channel->wrapper->key);
+    free(channel->wrapper);
+  }
+  // Set channel->wrapper to NULL to avoid call close twice for the same
+  // channel.
+  channel->wrapper = NULL;
+  gpr_mu_unlock(&global_persistent_list_mu);
 }
 
 // Delete an entry from the persistent list
@@ -435,6 +525,7 @@ void php_grpc_delete_persistent_list_entry(char *key, php_grpc_int key_len
     le = (channel_persistent_le_t *)rsrc->ptr;
     le->channel = NULL;
     php_grpc_zend_hash_del(&EG(persistent_list), key, key_len+1);
+    free(le);
   }
   gpr_mu_unlock(&global_persistent_list_mu);
 }