From da95c7b80eeef4f9875ecefb2111f0965ce29303 Mon Sep 17 00:00:00 2001
From: Hannah Shi <hannahshisfb@gmail.com>
Date: Wed, 29 Apr 2020 22:10:34 +0000
Subject: [PATCH] address comments, refactor & add release lock

---
 src/php/ext/grpc/channel.c  | 12 +++---------
 src/php/ext/grpc/channel.h  |  2 --
 src/php/ext/grpc/php_grpc.c | 36 ++++++++++++++----------------------
 3 files changed, 17 insertions(+), 33 deletions(-)

diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c
index 304ed932868..059e8333db1 100644
--- a/src/php/ext/grpc/channel.c
+++ b/src/php/ext/grpc/channel.c
@@ -586,13 +586,6 @@ void php_grpc_delete_persistent_list_entry(char *key, php_grpc_int key_len
   gpr_mu_unlock(&global_persistent_list_mu);
 }
 
-// Clean all channels in the persistent list
-// Called at post fork
-void php_grpc_clean_persistent_list(TSRMLS_D) {
-  zend_hash_clean(&grpc_persistent_list);
-  zend_hash_clean(&grpc_target_upper_bound_map);
-}
-
 // A destructor associated with each list entry from the persistent list
 static void php_grpc_channel_plink_dtor(php_grpc_zend_resource *rsrc
                                         TSRMLS_DC) {
@@ -622,11 +615,12 @@ static void php_grpc_target_bound_dtor(php_grpc_zend_resource *rsrc
 #ifdef GRPC_PHP_DEBUG
 
 /**
-* Clean all channels in the persistent
+* Clean all channels in the persistent. Test only.
 * @return void
 */
 PHP_METHOD(Channel, cleanPersistentList) {
-  php_grpc_clean_persistent_list(TSRMLS_C);
+  zend_hash_clean(&grpc_persistent_list);
+  zend_hash_clean(&grpc_target_upper_bound_map);
 }
 
 char *grpc_connectivity_state_name(grpc_connectivity_state state) {
diff --git a/src/php/ext/grpc/channel.h b/src/php/ext/grpc/channel.h
index 373e9877ea4..ce17c4a58a7 100644
--- a/src/php/ext/grpc/channel.h
+++ b/src/php/ext/grpc/channel.h
@@ -60,8 +60,6 @@ void generate_sha1_str(char *sha1str, char *str, php_grpc_int len);
 void php_grpc_delete_persistent_list_entry(char *key, php_grpc_int key_len
                                            TSRMLS_DC);
 
-void php_grpc_clean_persistent_list(TSRMLS_D);
-
 typedef struct _channel_persistent_le {
   grpc_channel_wrapper *channel;
 } channel_persistent_le_t;
diff --git a/src/php/ext/grpc/php_grpc.c b/src/php/ext/grpc/php_grpc.c
index c2d0fac0f19..87099f47a9e 100644
--- a/src/php/ext/grpc/php_grpc.c
+++ b/src/php/ext/grpc/php_grpc.c
@@ -162,35 +162,30 @@ void destroy_grpc_channels() {
   PHP_GRPC_HASH_FOREACH_END()
 }
 
-void restart_channels() {
-  zval *data;
-  PHP_GRPC_HASH_FOREACH_VAL_START(&grpc_persistent_list, data)
-    php_grpc_zend_resource *rsrc  =
-                (php_grpc_zend_resource*) PHP_GRPC_HASH_VALPTR_TO_VAL(data)
-    if (rsrc == NULL) {
-      break;
-    }
-    channel_persistent_le_t* le = rsrc->ptr;
-
-    wrapped_grpc_channel wrapped_channel;
-    wrapped_channel.wrapper = le->channel;
-    grpc_channel_wrapper *channel = wrapped_channel.wrapper;
-    create_new_channel(&wrapped_channel, channel->target, channel->args,
-                       channel->creds);
-    gpr_mu_unlock(&channel->mu);
-  PHP_GRPC_HASH_FOREACH_END()
-}
-
 void prefork() {
   acquire_persistent_locks();
 }
 
+// Clean all channels in the persistent list
+// Called at post fork
+void php_grpc_clean_persistent_list(TSRMLS_D) {
+    zend_hash_clean(&grpc_persistent_list);
+    zend_hash_destroy(&grpc_persistent_list);
+    zend_hash_clean(&grpc_target_upper_bound_map);
+    zend_hash_destroy(&grpc_target_upper_bound_map);
+}
+
 void postfork_child() {
   TSRMLS_FETCH();
 
   // loop through persistent list and destroy all underlying grpc_channel objs
   destroy_grpc_channels();
 
+  release_persistent_locks();
+  
+  // clean all channels in the persistent list
+  php_grpc_clean_persistent_list(TSRMLS_C);
+
   // clear completion queue
   grpc_php_shutdown_completion_queue(TSRMLS_C);
 
@@ -205,9 +200,6 @@ void postfork_child() {
   // restart grpc_core
   grpc_init();
   grpc_php_init_completion_queue(TSRMLS_C);
-
-  // clean all channels in the persistent list
-  php_grpc_clean_persistent_list(TSRMLS_C);
 }
 
 void postfork_parent() {