diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c index ed30888c43a..f3a03ace093 100644 --- a/src/php/ext/grpc/channel.c +++ b/src/php/ext/grpc/channel.c @@ -54,6 +54,7 @@ static zend_object_handlers channel_ce_handlers; #endif static gpr_mu global_persistent_list_mu; int le_plink; +extern HashTable grpc_persistent_list; /* Frees and destroys an instance of wrapped_grpc_channel */ PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel) @@ -68,17 +69,21 @@ PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel) 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, + if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&grpc_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){ + if (p->wrapper->creds_hashstr != NULL) { free(p->wrapper->creds_hashstr); p->wrapper->creds_hashstr = NULL; } free(p->wrapper->key); + p->wrapper->wrapped = NULL; + p->wrapper->target = NULL; + p->wrapper->args_hashstr = NULL; + p->wrapper->key = NULL; } gpr_mu_unlock(&global_persistent_list_mu); } @@ -188,9 +193,10 @@ void create_and_add_channel_to_persistent_list( create_channel(channel, target, args, creds); le->channel = channel->wrapper; + le->ref_count = 1; new_rsrc.ptr = le; gpr_mu_lock(&global_persistent_list_mu); - PHP_GRPC_PERSISTENT_LIST_UPDATE(&EG(persistent_list), key, key_len, + PHP_GRPC_PERSISTENT_LIST_UPDATE(&grpc_persistent_list, key, key_len, (void *)&new_rsrc); gpr_mu_unlock(&global_persistent_list_mu); } @@ -311,7 +317,7 @@ PHP_METHOD(Channel, __construct) { // object, there is no way we can tell them apart. Do NOT persist // them. They should be individually destroyed. create_channel(channel, target, args, creds); - } else if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&EG(persistent_list), key, + } else if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&grpc_persistent_list, key, key_len, rsrc))) { create_and_add_channel_to_persistent_list( channel, target, args, creds, key, key_len TSRMLS_CC); @@ -327,7 +333,7 @@ PHP_METHOD(Channel, __construct) { channel, target, args, creds, key, key_len TSRMLS_CC); } else { efree(args.args); - if (channel->wrapper->creds_hashstr != NULL){ + if (channel->wrapper->creds_hashstr != NULL) { free(channel->wrapper->creds_hashstr); channel->wrapper->creds_hashstr = NULL; } @@ -336,6 +342,7 @@ PHP_METHOD(Channel, __construct) { free(channel->wrapper->target); free(channel->wrapper->args_hashstr); free(channel->wrapper); + le->ref_count += 1; channel->wrapper = le->channel; channel->wrapper->ref_count += 1; } @@ -456,10 +463,10 @@ PHP_METHOD(Channel, close) { 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; - } + free(channel->wrapper->creds_hashstr); + channel->wrapper->creds_hashstr = NULL; + channel->wrapper->target = NULL; + channel->wrapper->args_hashstr = NULL; channel->wrapper->wrapped = NULL; channel->wrapper->is_valid = false; @@ -469,7 +476,7 @@ PHP_METHOD(Channel, close) { } } channel->wrapper->ref_count -= 1; - if(channel->wrapper->ref_count == 0){ + 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; @@ -494,12 +501,12 @@ void php_grpc_delete_persistent_list_entry(char *key, php_grpc_int key_len TSRMLS_DC) { php_grpc_zend_resource *rsrc; gpr_mu_lock(&global_persistent_list_mu); - if (PHP_GRPC_PERSISTENT_LIST_FIND(&EG(persistent_list), key, + if (PHP_GRPC_PERSISTENT_LIST_FIND(&grpc_persistent_list, key, key_len, rsrc)) { channel_persistent_le_t *le; le = (channel_persistent_le_t *)rsrc->ptr; le->channel = NULL; - php_grpc_zend_hash_del(&EG(persistent_list), key, key_len+1); + php_grpc_zend_hash_del(&grpc_persistent_list, key, key_len+1); free(le); } gpr_mu_unlock(&global_persistent_list_mu); @@ -509,27 +516,71 @@ void php_grpc_delete_persistent_list_entry(char *key, php_grpc_int key_len static void php_grpc_channel_plink_dtor(php_grpc_zend_resource *rsrc TSRMLS_DC) { channel_persistent_le_t *le = (channel_persistent_le_t *)rsrc->ptr; + if (le == NULL) { + return; + } if (le->channel != NULL) { gpr_mu_lock(&le->channel->mu); if (le->channel->wrapped != NULL) { grpc_channel_destroy(le->channel->wrapped); - free(le->channel->target); free(le->channel->args_hashstr); le->channel->wrapped = NULL; le->channel->target = NULL; le->channel->args_hashstr = NULL; + free(le->channel->key); + le->channel->key = NULL; } - free(le->channel->key); - le->channel->key = NULL; gpr_mu_unlock(&le->channel->mu); - gpr_mu_destroy(&le->channel->mu); - free(le->channel); - le->channel = NULL; - free(le); - le = NULL; } } +/** + * Clean all channels in the persistent. + * @return void + */ +PHP_METHOD(Channel, cleanPersistentList) { + zend_hash_clean(&grpc_persistent_list); +} + +/** + * Return an array of persistent list. + * @return array + */ +PHP_METHOD(Channel, getPersistentList) { + array_init(return_value); + 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; + zval* ret_arr; + PHP_GRPC_MAKE_STD_ZVAL(ret_arr); + array_init(ret_arr); + // Info about the target + PHP_GRPC_ADD_STRING_TO_ARRAY(ret_arr, "target", + sizeof("target"), le->channel->target, true); + // Info about key + PHP_GRPC_ADD_STRING_TO_ARRAY(ret_arr, "key", + sizeof("key"), le->channel->key, true); + // Info about persistent channel ref_count + PHP_GRPC_ADD_LONG_TO_ARRAY(ret_arr, "ref_count", + sizeof("ref_count"), le->ref_count); + // Info about connectivity status + int state = + grpc_channel_check_connectivity_state(le->channel->wrapped, (int)0); + // It should be set to 'true' in PHP 5.6.33 + PHP_GRPC_ADD_LONG_TO_ARRAY(ret_arr, "connectivity_status", + sizeof("connectivity_status"), state); + // Info about the channel is closed or not + PHP_GRPC_ADD_BOOL_TO_ARRAY(ret_arr, "is_valid", + sizeof("is_valid"), le->channel->is_valid); + add_assoc_zval(return_value, le->channel->target, ret_arr); + PHP_GRPC_HASH_FOREACH_END() +} + ZEND_BEGIN_ARG_INFO_EX(arginfo_construct, 0, 0, 2) ZEND_ARG_INFO(0, target) ZEND_ARG_INFO(0, args) @@ -550,6 +601,12 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_close, 0, 0, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_cleanPersistentList, 0, 0, 0) +ZEND_END_ARG_INFO() + +ZEND_BEGIN_ARG_INFO_EX(arginfo_getPersistentList, 0, 0, 0) +ZEND_END_ARG_INFO() + static zend_function_entry channel_methods[] = { PHP_ME(Channel, __construct, arginfo_construct, ZEND_ACC_PUBLIC | ZEND_ACC_CTOR) @@ -561,6 +618,10 @@ static zend_function_entry channel_methods[] = { ZEND_ACC_PUBLIC) PHP_ME(Channel, close, arginfo_close, ZEND_ACC_PUBLIC) + PHP_ME(Channel, cleanPersistentList, arginfo_cleanPersistentList, + ZEND_ACC_PUBLIC) + PHP_ME(Channel, getPersistentList, arginfo_getPersistentList, + ZEND_ACC_PUBLIC) PHP_FE_END }; @@ -572,6 +633,8 @@ GRPC_STARTUP_FUNCTION(channel) { gpr_mu_init(&global_persistent_list_mu); le_plink = zend_register_list_destructors_ex( NULL, php_grpc_channel_plink_dtor, "Persistent Channel", module_number); + zend_hash_init_ex(&grpc_persistent_list, 20, NULL, + EG(persistent_list).pDestructor, 1, 0); PHP_GRPC_INIT_HANDLER(wrapped_grpc_channel, channel_ce_handlers); return SUCCESS; } diff --git a/src/php/ext/grpc/channel.h b/src/php/ext/grpc/channel.h index 86bfdea51a2..807880534e5 100644 --- a/src/php/ext/grpc/channel.h +++ b/src/php/ext/grpc/channel.h @@ -84,6 +84,7 @@ void php_grpc_delete_persistent_list_entry(char *key, php_grpc_int key_len typedef struct _channel_persistent_le { grpc_channel_wrapper *channel; + size_t ref_count; } channel_persistent_le_t; diff --git a/src/php/ext/grpc/php7_wrapper.h b/src/php/ext/grpc/php7_wrapper.h index 2f4a53611c3..d4fa859249e 100644 --- a/src/php/ext/grpc/php7_wrapper.h +++ b/src/php/ext/grpc/php7_wrapper.h @@ -38,6 +38,12 @@ #define PHP_GRPC_MAKE_STD_ZVAL(pzv) MAKE_STD_ZVAL(pzv) #define PHP_GRPC_FREE_STD_ZVAL(pzv) #define PHP_GRPC_DELREF(zv) Z_DELREF_P(zv) +#define PHP_GRPC_ADD_STRING_TO_ARRAY(val, key, key_len, str, dup) \ + add_assoc_string_ex(val, key, key_len , str, dup); +#define PHP_GRPC_ADD_LONG_TO_ARRAY(val, key, key_len, str) \ + add_assoc_long_ex(val, key, key_len, str); +#define PHP_GRPC_ADD_BOOL_TO_ARRAY(val, key, key_len, str) \ + add_assoc_bool_ex(val, key, key_len, str); #define RETURN_DESTROY_ZVAL(val) \ RETURN_ZVAL(val, false /* Don't execute copy constructor */, \ @@ -88,6 +94,9 @@ 0, NULL); \ data = *tmp##key; +#define PHP_GRPC_HASH_VALPTR_TO_VAL(data) \ + &data; + #define PHP_GRPC_HASH_FOREACH_LONG_KEY_VAL_START(ht, key, key_type, index,\ data) \ zval **tmp##key = NULL; \ @@ -128,6 +137,8 @@ static inline int php_grpc_zend_hash_find(HashTable *ht, char *key, int len, #define PHP_GRPC_PERSISTENT_LIST_UPDATE(plist, key, len, rsrc) \ zend_hash_update(plist, key, len+1, rsrc, sizeof(php_grpc_zend_resource), \ NULL) +#define PHP_GRPC_PERSISTENT_LIST_SIZE(plist) \ + plist.nTableSize #define PHP_GRPC_GET_CLASS_ENTRY(object) zend_get_class_entry(object TSRMLS_CC) @@ -152,6 +163,12 @@ static inline int php_grpc_zend_hash_find(HashTable *ht, char *key, int len, pzv = (zval *)emalloc(sizeof(zval)); #define PHP_GRPC_FREE_STD_ZVAL(pzv) efree(pzv); #define PHP_GRPC_DELREF(zv) +#define PHP_GRPC_ADD_STRING_TO_ARRAY(val, key, key_len, str, dup) \ + add_assoc_string_ex(val, key, key_len - 1, str); +#define PHP_GRPC_ADD_LONG_TO_ARRAY(val, key, key_len, str) \ + add_assoc_long_ex(val, key, key_len - 1, str); +#define PHP_GRPC_ADD_BOOL_TO_ARRAY(val, key, key_len, str) \ + add_assoc_bool_ex(val, key, key_len - 1, str); #define RETURN_DESTROY_ZVAL(val) \ RETVAL_ZVAL(val, false /* Don't execute copy constructor */, \ @@ -193,6 +210,9 @@ static inline int php_grpc_zend_hash_find(HashTable *ht, char *key, int len, if ((zs_##key) == NULL) {key = NULL; key_type = HASH_KEY_IS_LONG;} \ else {key = (zs_##key)->val; key_type = HASH_KEY_IS_STRING;} +#define PHP_GRPC_HASH_VALPTR_TO_VAL(data) \ + Z_PTR_P(data); + #define PHP_GRPC_HASH_FOREACH_LONG_KEY_VAL_START(ht, key, key_type, index, \ data) \ zend_string *(zs_##key); \ @@ -230,6 +250,8 @@ static inline int php_grpc_zend_hash_del(HashTable *ht, char *key, int len) { #define PHP_GRPC_PERSISTENT_LIST_UPDATE(plist, key, len, rsrc) \ zend_hash_str_update_mem(plist, key, len, rsrc, \ sizeof(php_grpc_zend_resource)) +#define PHP_GRPC_PERSISTENT_LIST_SIZE(plist) \ + zend_array_count(plist) #define PHP_GRPC_GET_CLASS_ENTRY(object) Z_OBJ_P(object)->ce diff --git a/src/php/ext/grpc/php_grpc.c b/src/php/ext/grpc/php_grpc.c index 5971babc00e..883ee6f3e50 100644 --- a/src/php/ext/grpc/php_grpc.c +++ b/src/php/ext/grpc/php_grpc.c @@ -36,7 +36,7 @@ ZEND_DECLARE_MODULE_GLOBALS(grpc) static PHP_GINIT_FUNCTION(grpc); - +HashTable grpc_persistent_list; /* {{{ grpc_functions[] * * Every user visible function must have an entry in grpc_functions[]. @@ -240,6 +240,8 @@ PHP_MSHUTDOWN_FUNCTION(grpc) { // WARNING: This function IS being called by PHP when the extension // is unloaded but the logs were somehow suppressed. if (GRPC_G(initialized)) { + zend_hash_clean(&grpc_persistent_list); + zend_hash_destroy(&grpc_persistent_list); grpc_shutdown_timeval(TSRMLS_C); grpc_php_shutdown_completion_queue(TSRMLS_C); grpc_shutdown(); @@ -256,7 +258,6 @@ PHP_MINFO_FUNCTION(grpc) { php_info_print_table_row(2, "grpc support", "enabled"); php_info_print_table_row(2, "grpc module version", PHP_GRPC_VERSION); php_info_print_table_end(); - /* Remove comments if you have entries in php.ini DISPLAY_INI_ENTRIES(); */ diff --git a/src/php/tests/unit_tests/CallCredentials2Test.php b/src/php/tests/unit_tests/CallCredentials2Test.php index 1c7e0c0ff6f..a462bfff562 100644 --- a/src/php/tests/unit_tests/CallCredentials2Test.php +++ b/src/php/tests/unit_tests/CallCredentials2Test.php @@ -46,6 +46,9 @@ class CallCredentials2Test extends PHPUnit_Framework_TestCase { unset($this->channel); unset($this->server); + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function callbackFunc($context) diff --git a/src/php/tests/unit_tests/CallCredentialsTest.php b/src/php/tests/unit_tests/CallCredentialsTest.php index 4b5721d76ac..31046e63957 100644 --- a/src/php/tests/unit_tests/CallCredentialsTest.php +++ b/src/php/tests/unit_tests/CallCredentialsTest.php @@ -52,6 +52,9 @@ class CallCredentialsTest extends PHPUnit_Framework_TestCase { unset($this->channel); unset($this->server); + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function callbackFunc($context) diff --git a/src/php/tests/unit_tests/CallTest.php b/src/php/tests/unit_tests/CallTest.php index c5e1890a98b..38c36ed19a9 100644 --- a/src/php/tests/unit_tests/CallTest.php +++ b/src/php/tests/unit_tests/CallTest.php @@ -38,6 +38,9 @@ class CallTest extends PHPUnit_Framework_TestCase public function tearDown() { $this->channel->close(); + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function testConstructor() diff --git a/src/php/tests/unit_tests/ChannelTest.php b/src/php/tests/unit_tests/ChannelTest.php index 5baff1fbd93..63d4193a8be 100644 --- a/src/php/tests/unit_tests/ChannelTest.php +++ b/src/php/tests/unit_tests/ChannelTest.php @@ -28,6 +28,9 @@ class ChannelTest extends PHPUnit_Framework_TestCase if (!empty($this->channel)) { $this->channel->close(); } + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function testInsecureCredentials() @@ -380,6 +383,11 @@ class ChannelTest extends PHPUnit_Framework_TestCase // close channel1 $this->channel1->close(); + // channel2 is now in SHUTDOWN state + $state = $this->channel2->getConnectivityState(); + $this->assertEquals(GRPC\CHANNEL_FATAL_FAILURE, $state); + + // calling it again will result in an exception because the // channel is already closed $state = $this->channel2->getConnectivityState(); } diff --git a/src/php/tests/unit_tests/EndToEndTest.php b/src/php/tests/unit_tests/EndToEndTest.php index b54f1d87c9f..71a56d2b6eb 100644 --- a/src/php/tests/unit_tests/EndToEndTest.php +++ b/src/php/tests/unit_tests/EndToEndTest.php @@ -29,6 +29,9 @@ class EndToEndTest extends PHPUnit_Framework_TestCase public function tearDown() { $this->channel->close(); + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function testSimpleRequestBody() diff --git a/src/php/tests/unit_tests/PersistentChannelTest.php b/src/php/tests/unit_tests/PersistentChannelTest.php new file mode 100644 index 00000000000..31c6bbaf402 --- /dev/null +++ b/src/php/tests/unit_tests/PersistentChannelTest.php @@ -0,0 +1,111 @@ +cleanPersistentList(); + } + + public function waitUntilNotIdle($channel) { + for ($i = 0; $i < 10; $i++) { + $now = Grpc\Timeval::now(); + $deadline = $now->add(new Grpc\Timeval(1000)); + if ($channel->watchConnectivityState(GRPC\CHANNEL_IDLE, + $deadline)) { + return true; + } + } + $this->assertTrue(false); + } + + public function testPersistentChennelCreateOneChannel() + { + $this->channel1 = new Grpc\Channel('localhost:1', []); + $plist = $this->channel1->getPersistentList(); + $this->assertEquals($plist['localhost:1']['target'], 'localhost:1'); + $this->assertArrayHasKey('localhost:1', $plist); + $this->assertEquals($plist['localhost:1']['ref_count'], 1); + $this->assertEquals($plist['localhost:1']['connectivity_status'], + GRPC\CHANNEL_IDLE); + $this->assertEquals($plist['localhost:1']['is_valid'], 1); + $this->channel1->close(); + } + + public function testPersistentChennelStatusChange() + { + $this->channel1 = new Grpc\Channel('localhost:1', []); + $plist = $this->channel1->getPersistentList(); + $this->assertEquals($plist['localhost:1']['connectivity_status'], + GRPC\CHANNEL_IDLE); + $this->assertEquals($plist['localhost:1']['is_valid'], 1); + $state = $this->channel1->getConnectivityState(true); + + $this->waitUntilNotIdle($this->channel1); + $plist = $this->channel1->getPersistentList(); + $this->assertEquals($plist['localhost:1']['connectivity_status'], + GRPC\CHANNEL_CONNECTING); + $this->assertEquals($plist['localhost:1']['is_valid'], 1); + + $this->channel1->close(); + } + + public function testPersistentChennelCloseChannel() + { + $this->channel1 = new Grpc\Channel('localhost:1', []); + $plist = $this->channel1->getPersistentList(); + $this->assertEquals($plist['localhost:1']['ref_count'], 1); + $this->channel1->close(); + $plist = $this->channel1->getPersistentList(); + $this->assertArrayNotHasKey('localhost:1', $plist); + } + + public function testPersistentChannelSameHost() + { + $this->channel1 = new Grpc\Channel('localhost:1', []); + $this->channel2 = new Grpc\Channel('localhost:1', []); + //ref_count should be 2 + $plist = $this->channel1->getPersistentList(); + $this->assertArrayHasKey('localhost:1', $plist); + $this->assertEquals($plist['localhost:1']['ref_count'], 2); + $this->channel1->close(); + $this->channel2->close(); + } + + public function testPersistentChannelDifferentHost() + { + $this->channel1 = new Grpc\Channel('localhost:1', []); + $this->channel2 = new Grpc\Channel('localhost:2', []); + $plist = $this->channel1->getPersistentList(); + $this->assertArrayHasKey('localhost:1', $plist); + $this->assertArrayHasKey('localhost:2', $plist); + $this->assertEquals($plist['localhost:1']['ref_count'], 1); + $this->assertEquals($plist['localhost:2']['ref_count'], 1); + $this->channel1->close(); + $this->channel2->close(); + } + +} diff --git a/src/php/tests/unit_tests/SecureEndToEndTest.php b/src/php/tests/unit_tests/SecureEndToEndTest.php index dff4e878ea1..e358abe2d28 100644 --- a/src/php/tests/unit_tests/SecureEndToEndTest.php +++ b/src/php/tests/unit_tests/SecureEndToEndTest.php @@ -44,6 +44,9 @@ class SecureEndToEndTest extends PHPUnit_Framework_TestCase public function tearDown() { $this->channel->close(); + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function testSimpleRequestBody() diff --git a/src/php/tests/unit_tests/ServerTest.php b/src/php/tests/unit_tests/ServerTest.php index fee9f1e6448..d18feecefec 100644 --- a/src/php/tests/unit_tests/ServerTest.php +++ b/src/php/tests/unit_tests/ServerTest.php @@ -27,6 +27,9 @@ class ServerTest extends PHPUnit_Framework_TestCase public function tearDown() { unset($this->server); + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function testConstructorWithNull() diff --git a/src/php/tests/unit_tests/TimevalTest.php b/src/php/tests/unit_tests/TimevalTest.php index bc307ef7f1e..be023adace6 100644 --- a/src/php/tests/unit_tests/TimevalTest.php +++ b/src/php/tests/unit_tests/TimevalTest.php @@ -25,6 +25,9 @@ class TimevalTest extends PHPUnit_Framework_TestCase public function tearDown() { unset($this->time); + $channel_clean_persistent = + new Grpc\Channel('localhost:50010', []); + $channel_clean_persistent->cleanPersistentList(); } public function testConstructorWithInt()