diff --git a/package.xml b/package.xml
index 6e50badfc28..d5bd4d062a4 100644
--- a/package.xml
+++ b/package.xml
@@ -22,13 +22,9 @@
Apache 2.0
-- Fixed some memory leaks #9559, #10996
-- Disabled cares dependency from gRPC C Core #10940
-- De-coupled protobuf dependency #11112
-- Fixed extension reported version #10842
-- Added config.w32 for Windows support #8161
-- Fixed PHP distrib test after cc files were added #11193
-- Fixed protoc plugin comment escape bug #11025
+- Channel are now by default persistent #11878
+- Some bug fixes from 1.4 branch #12109, #12123
+- Fixed hang bug when fork() was used #11814
diff --git a/src/php/ext/grpc/call_credentials.c b/src/php/ext/grpc/call_credentials.c
index f46091d7098..1eee8645df9 100644
--- a/src/php/ext/grpc/call_credentials.c
+++ b/src/php/ext/grpc/call_credentials.c
@@ -179,7 +179,7 @@ void plugin_get_metadata(void *ptr, grpc_auth_metadata_context context,
grpc_metadata_array metadata;
bool cleanup = true;
- if (Z_TYPE_P(retval) != IS_ARRAY) {
+ if (retval == NULL || Z_TYPE_P(retval) != IS_ARRAY) {
cleanup = false;
code = GRPC_STATUS_INVALID_ARGUMENT;
} else if (!create_metadata_array(retval, &metadata)) {
diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c
index f1187e8722f..dc3acc89bb7 100644
--- a/src/php/ext/grpc/channel.c
+++ b/src/php/ext/grpc/channel.c
@@ -66,6 +66,8 @@ PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel)
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);
}
gpr_mu_unlock(&global_persistent_list_mu);
}
@@ -158,7 +160,7 @@ void create_and_add_channel_to_persistent_list(
grpc_channel_args args,
wrapped_grpc_channel_credentials *creds,
char *key,
- php_grpc_int key_len) {
+ php_grpc_int key_len TSRMLS_DC) {
php_grpc_zend_resource new_rsrc;
channel_persistent_le_t *le;
// this links each persistent list entry to a destructor
@@ -187,10 +189,8 @@ void create_and_add_channel_to_persistent_list(
* credentials.
*
* If the $args array contains a "force_new" key mapping to a boolean value
- * of "true", a new underlying grpc_channel will be created regardless. If
- * there are any opened channels on the same hostname, user must manually
- * call close() on those dangling channels before the end of the PHP
- * script.
+ * of "true", a new and separate underlying grpc_channel will be created
+ * and returned. This will not affect existing channels.
*
* @param string $target The hostname to associate with this channel
* @param array $args_array The arguments to pass to the Channel
@@ -273,19 +273,15 @@ PHP_METHOD(Channel, __construct) {
}
channel->wrapper = malloc(sizeof(grpc_channel_wrapper));
channel->wrapper->key = key;
- channel->wrapper->target = target;
- channel->wrapper->args_hashstr = sha1str;
+ channel->wrapper->target = strdup(target);
+ channel->wrapper->args_hashstr = strdup(sha1str);
if (creds != NULL && creds->hashstr != NULL) {
channel->wrapper->creds_hashstr = creds->hashstr;
}
gpr_mu_init(&channel->wrapper->mu);
smart_str_free(&buf);
- if (force_new) {
- php_grpc_delete_persistent_list_entry(key, key_len TSRMLS_CC);
- }
-
- if (creds != NULL && creds->has_call_creds) {
+ if (force_new || (creds != NULL && creds->has_call_creds)) {
// If the ChannelCredentials object was composed with a CallCredentials
// object, there is no way we can tell them apart. Do NOT persist
// them. They should be individually destroyed.
@@ -293,7 +289,7 @@ PHP_METHOD(Channel, __construct) {
} else if (!(PHP_GRPC_PERSISTENT_LIST_FIND(&EG(persistent_list), key,
key_len, rsrc))) {
create_and_add_channel_to_persistent_list(
- channel, target, args, creds, key, key_len);
+ channel, target, args, creds, key, key_len TSRMLS_CC);
} else {
// Found a previously stored channel in the persistent list
channel_persistent_le_t *le = (channel_persistent_le_t *)rsrc->ptr;
@@ -303,7 +299,7 @@ PHP_METHOD(Channel, __construct) {
strcmp(creds->hashstr, le->channel->creds_hashstr) != 0)) {
// somehow hash collision
create_and_add_channel_to_persistent_list(
- channel, target, args, creds, key, key_len);
+ channel, target, args, creds, key, key_len TSRMLS_CC);
} else {
channel->wrapper = le->channel;
}
@@ -416,12 +412,14 @@ PHP_METHOD(Channel, close) {
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);
+ php_grpc_delete_persistent_list_entry(channel->wrapper->key,
+ strlen(channel->wrapper->key)
+ TSRMLS_CC);
+ }
gpr_mu_unlock(&channel->wrapper->mu);
}
@@ -449,12 +447,11 @@ static void php_grpc_channel_plink_dtor(php_grpc_zend_resource *rsrc
gpr_mu_lock(&le->channel->mu);
if (le->channel->wrapped != NULL) {
grpc_channel_destroy(le->channel->wrapped);
- free(le->channel->key);
- free(le->channel);
+ free(le->channel->target);
+ free(le->channel->args_hashstr);
}
gpr_mu_unlock(&le->channel->mu);
}
- free(le);
}
ZEND_BEGIN_ARG_INFO_EX(arginfo_construct, 0, 0, 2)
@@ -496,6 +493,7 @@ GRPC_STARTUP_FUNCTION(channel) {
INIT_CLASS_ENTRY(ce, "Grpc\\Channel", channel_methods);
ce.create_object = create_wrapped_grpc_channel;
grpc_ce_channel = zend_register_internal_class(&ce TSRMLS_CC);
+ gpr_mu_init(&global_persistent_list_mu);
le_plink = zend_register_list_destructors_ex(
NULL, php_grpc_channel_plink_dtor, "Persistent Channel", module_number);
PHP_GRPC_INIT_HANDLER(wrapped_grpc_channel, channel_ce_handlers);
diff --git a/src/php/ext/grpc/channel_credentials.c b/src/php/ext/grpc/channel_credentials.c
index 19e1cefb6f5..86e4f46c67a 100644
--- a/src/php/ext/grpc/channel_credentials.c
+++ b/src/php/ext/grpc/channel_credentials.c
@@ -135,6 +135,8 @@ PHP_METHOD(ChannelCredentials, createSsl) {
pem_key_cert_pair.private_key = pem_key_cert_pair.cert_chain = NULL;
+ grpc_set_ssl_roots_override_callback(get_ssl_roots_override);
+
/* "|s!s!s!" == 3 optional nullable strings */
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|s!s!s!",
&pem_root_certs, &root_certs_length,
@@ -148,7 +150,7 @@ PHP_METHOD(ChannelCredentials, createSsl) {
}
php_grpc_int hashkey_len = root_certs_length + cert_chain_length;
- char hashkey[hashkey_len];
+ char *hashkey = emalloc(hashkey_len);
if (root_certs_length > 0) {
strcpy(hashkey, pem_root_certs);
}
@@ -164,6 +166,7 @@ PHP_METHOD(ChannelCredentials, createSsl) {
pem_key_cert_pair.private_key == NULL ? NULL : &pem_key_cert_pair, NULL);
zval *creds_object = grpc_php_wrap_channel_credentials(creds, hashstr, false
TSRMLS_CC);
+ efree(hashkey);
RETURN_DESTROY_ZVAL(creds_object);
}
@@ -177,6 +180,8 @@ PHP_METHOD(ChannelCredentials, createComposite) {
zval *cred1_obj;
zval *cred2_obj;
+ grpc_set_ssl_roots_override_callback(get_ssl_roots_override);
+
/* "OO" == 2 Objects */
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "OO", &cred1_obj,
grpc_ce_channel_credentials, &cred2_obj,
@@ -245,7 +250,6 @@ void grpc_init_channel_credentials(TSRMLS_D) {
zend_class_entry ce;
INIT_CLASS_ENTRY(ce, "Grpc\\ChannelCredentials",
channel_credentials_methods);
- grpc_set_ssl_roots_override_callback(get_ssl_roots_override);
ce.create_object = create_wrapped_grpc_channel_credentials;
grpc_ce_channel_credentials = zend_register_internal_class(&ce TSRMLS_CC);
PHP_GRPC_INIT_HANDLER(wrapped_grpc_channel_credentials,
diff --git a/src/php/ext/grpc/php_grpc.c b/src/php/ext/grpc/php_grpc.c
index a96daf7d9b8..4ed56de9930 100644
--- a/src/php/ext/grpc/php_grpc.c
+++ b/src/php/ext/grpc/php_grpc.c
@@ -34,7 +34,8 @@
#include
#include "php_grpc.h"
-// ZEND_DECLARE_MODULE_GLOBALS(grpc)
+ZEND_DECLARE_MODULE_GLOBALS(grpc)
+static PHP_GINIT_FUNCTION(grpc);
/* {{{ grpc_functions[]
*
@@ -55,13 +56,17 @@ zend_module_entry grpc_module_entry = {
grpc_functions,
PHP_MINIT(grpc),
PHP_MSHUTDOWN(grpc),
- NULL,
+ PHP_RINIT(grpc),
NULL,
PHP_MINFO(grpc),
#if ZEND_MODULE_API_NO >= 20010901
PHP_GRPC_VERSION,
#endif
- STANDARD_MODULE_PROPERTIES};
+ PHP_MODULE_GLOBALS(grpc),
+ PHP_GINIT(grpc),
+ NULL,
+ NULL,
+ STANDARD_MODULE_PROPERTIES_EX};
/* }}} */
#ifdef COMPILE_DL_GRPC
@@ -99,7 +104,6 @@ PHP_MINIT_FUNCTION(grpc) {
REGISTER_INI_ENTRIES();
*/
/* Register call error constants */
- grpc_init();
REGISTER_LONG_CONSTANT("Grpc\\CALL_OK", GRPC_CALL_OK,
CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("Grpc\\CALL_ERROR", GRPC_CALL_ERROR,
@@ -227,7 +231,6 @@ PHP_MINIT_FUNCTION(grpc) {
grpc_init_channel_credentials(TSRMLS_C);
grpc_init_call_credentials(TSRMLS_C);
grpc_init_server_credentials(TSRMLS_C);
- grpc_php_init_completion_queue(TSRMLS_C);
return SUCCESS;
}
/* }}} */
@@ -240,9 +243,12 @@ PHP_MSHUTDOWN_FUNCTION(grpc) {
*/
// WARNING: This function IS being called by PHP when the extension
// is unloaded but the logs were somehow suppressed.
- grpc_shutdown_timeval(TSRMLS_C);
- grpc_php_shutdown_completion_queue(TSRMLS_C);
- grpc_shutdown();
+ if (GRPC_G(initialized)) {
+ grpc_shutdown_timeval(TSRMLS_C);
+ grpc_php_shutdown_completion_queue(TSRMLS_C);
+ grpc_shutdown();
+ GRPC_G(initialized) = 0;
+ }
return SUCCESS;
}
/* }}} */
@@ -259,6 +265,26 @@ PHP_MINFO_FUNCTION(grpc) {
*/
}
/* }}} */
+
+/* {{{ PHP_RINIT_FUNCTION
+ */
+PHP_RINIT_FUNCTION(grpc) {
+ if (!GRPC_G(initialized)) {
+ grpc_init();
+ grpc_php_init_completion_queue(TSRMLS_C);
+ GRPC_G(initialized) = 1;
+ }
+ return SUCCESS;
+}
+/* }}} */
+
+/* {{{ PHP_GINIT_FUNCTION
+ */
+static PHP_GINIT_FUNCTION(grpc) {
+ grpc_globals->initialized = 0;
+}
+/* }}} */
+
/* The previous line is meant for vim and emacs, so it can correctly fold and
unfold functions in source code. See the corresponding marks just before
function definition, where the functions purpose is also documented. Please
diff --git a/src/php/ext/grpc/php_grpc.h b/src/php/ext/grpc/php_grpc.h
index 32329178dc4..e30f011c395 100644
--- a/src/php/ext/grpc/php_grpc.h
+++ b/src/php/ext/grpc/php_grpc.h
@@ -49,14 +49,16 @@ PHP_MINIT_FUNCTION(grpc);
PHP_MSHUTDOWN_FUNCTION(grpc);
/* Displays information about the module */
PHP_MINFO_FUNCTION(grpc);
+/* Code that runs at request start */
+PHP_RINIT_FUNCTION(grpc);
/*
Declare any global variables you may need between the BEGIN
and END macros here:
-
+*/
ZEND_BEGIN_MODULE_GLOBALS(grpc)
+ zend_bool initialized;
ZEND_END_MODULE_GLOBALS(grpc)
-*/
/* In every utility function you add that needs to use variables
in php_grpc_globals, call TSRMLS_FETCH(); after declaring other
diff --git a/src/php/tests/unit_tests/ChannelTest.php b/src/php/tests/unit_tests/ChannelTest.php
index 400df0fb66d..c375a16269d 100644
--- a/src/php/tests/unit_tests/ChannelTest.php
+++ b/src/php/tests/unit_tests/ChannelTest.php
@@ -517,8 +517,6 @@ class ChannelTest extends PHPUnit_Framework_TestCase
$state = $this->channel2->getConnectivityState();
$this->assertEquals(GRPC\CHANNEL_IDLE, $state);
- // any dangling old connection to the same host must be
- // manually closed
$this->channel1->close();
$this->channel2->close();
}
@@ -529,6 +527,7 @@ class ChannelTest extends PHPUnit_Framework_TestCase
$this->channel1 = new Grpc\Channel('localhost:1', []);
$this->channel2 = new Grpc\Channel('localhost:1',
["force_new" => true]);
+ // channel3 shares with channel1
$this->channel3 = new Grpc\Channel('localhost:1', []);
// try to connect on channel2
@@ -540,29 +539,31 @@ class ChannelTest extends PHPUnit_Framework_TestCase
$state = $this->channel2->getConnectivityState();
$this->assertConnecting($state);
$state = $this->channel3->getConnectivityState();
- $this->assertConnecting($state);
+ $this->assertEquals(GRPC\CHANNEL_IDLE, $state);
$this->channel1->close();
$this->channel2->close();
}
+ /**
+ * @expectedException RuntimeException
+ */
public function testPersistentChannelForceNewOldChannelClose()
{
$this->channel1 = new Grpc\Channel('localhost:1', []);
$this->channel2 = new Grpc\Channel('localhost:1',
["force_new" => true]);
+ // channel3 shares with channel1
$this->channel3 = new Grpc\Channel('localhost:1', []);
$this->channel1->close();
$state = $this->channel2->getConnectivityState();
$this->assertEquals(GRPC\CHANNEL_IDLE, $state);
- $state = $this->channel3->getConnectivityState();
- $this->assertEquals(GRPC\CHANNEL_IDLE, $state);
- $this->channel2->close();
- $this->channel3->close();
+ // channel3 already closed
+ $state = $this->channel3->getConnectivityState();
}
public function testPersistentChannelForceNewNewChannelClose()
diff --git a/templates/package.xml.template b/templates/package.xml.template
index 27cc4d01a99..15da704a478 100644
--- a/templates/package.xml.template
+++ b/templates/package.xml.template
@@ -24,13 +24,9 @@
Apache 2.0
- - Fixed some memory leaks #9559, #10996
- - Disabled cares dependency from gRPC C Core #10940
- - De-coupled protobuf dependency #11112
- - Fixed extension reported version #10842
- - Added config.w32 for Windows support #8161
- - Fixed PHP distrib test after cc files were added #11193
- - Fixed protoc plugin comment escape bug #11025
+ - Channel are now by default persistent #11878
+ - Some bug fixes from 1.4 branch #12109, #12123
+ - Fixed hang bug when fork() was used #11814