PHP: upmerge from 1.4.x branch
diff --git a/src/php/ext/grpc/call_credentials.c b/src/php/ext/grpc/call_credentials.c
index f46091d..1eee864 100644
--- a/src/php/ext/grpc/call_credentials.c
+++ b/src/php/ext/grpc/call_credentials.c
@@ -179,7 +179,7 @@
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 f1187e8..dc3acc8 100644
--- a/src/php/ext/grpc/channel.c
+++ b/src/php/ext/grpc/channel.c
@@ -66,6 +66,8 @@
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 @@
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 @@
* 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 @@
}
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 @@
} 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 @@
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 @@
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 @@
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 @@
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 19e1cef..86e4f46 100644
--- a/src/php/ext/grpc/channel_credentials.c
+++ b/src/php/ext/grpc/channel_credentials.c
@@ -135,6 +135,8 @@
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_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 @@
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 @@
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 @@
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 a96daf7..4ed56de 100644
--- a/src/php/ext/grpc/php_grpc.c
+++ b/src/php/ext/grpc/php_grpc.c
@@ -34,7 +34,8 @@
#include <ext/standard/info.h>
#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 @@
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 @@
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 @@
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 @@
*/
// 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_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 3232917..e30f011 100644
--- a/src/php/ext/grpc/php_grpc.h
+++ b/src/php/ext/grpc/php_grpc.h
@@ -49,14 +49,16 @@
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_END_MODULE_GLOBALS(grpc)
*/
+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 400df0f..c375a16 100644
--- a/src/php/tests/unit_tests/ChannelTest.php
+++ b/src/php/tests/unit_tests/ChannelTest.php
@@ -517,8 +517,6 @@
$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 @@
$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 @@
$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()