Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

php: fix channel reuse and wrapper memory leak

Browse Source
pull/14149/head
Zhouyihai Ding 7 years ago
parent 2c5964fffc
commit 5b98d0be11
4 changed files with 126 additions and 24 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 135
      src/php/ext/grpc/channel.c
  2. 5
      src/php/ext/grpc/channel.h
  3. 8
      src/php/ext/grpc/channel_credentials.c
  4. 2
      src/php/ext/grpc/php7_wrapper.h

135
src/php/ext/grpc/channel.c
Unescape View File

@ -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()
@ -276,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);
@ -303,7 +352,17 @@ PHP_METHOD(Channel, __construct) {
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;
}
}
}
@ -323,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);
}
/**
@ -411,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
@ -437,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);
}

5
src/php/ext/grpc/channel.h
Unescape View File

@ -40,6 +40,11 @@ typedef struct _grpc_channel_wrapper {
char *args_hashstr;
char *creds_hashstr;
gpr_mu mu;
// is_valid is used to check the wrapped channel has been freed
// before to avoid double free.
bool is_valid;
// ref_count is used to let the last wrapper free related channel and key.
size_t ref_count;
} grpc_channel_wrapper;
/* Wrapper struct for grpc_channel that can be associated with a PHP object */

8
src/php/ext/grpc/channel_credentials.c
Unescape View File

@ -59,6 +59,7 @@ static grpc_ssl_roots_override_result get_ssl_roots_override(
PHP_GRPC_FREE_WRAPPED_FUNC_START(wrapped_grpc_channel_credentials)
if (p->wrapped != NULL) {
grpc_channel_credentials_release(p->wrapped);
p->wrapped = NULL;
}
PHP_GRPC_FREE_WRAPPED_FUNC_END()
@ -199,8 +200,13 @@ PHP_METHOD(ChannelCredentials, createComposite) {
grpc_channel_credentials *creds =
grpc_composite_channel_credentials_create(cred1->wrapped, cred2->wrapped,
NULL);
// wrapped_grpc_channel_credentials object should keeps it's own
// allocation. Otherwise it conflicts free hashstr with call.c.
php_grpc_int cred1_len = strlen(cred1->hashstr);
char *cred1_hashstr = malloc(cred1_len+1);
strcpy(cred1_hashstr, cred1->hashstr);
zval *creds_object =
grpc_php_wrap_channel_credentials(creds, cred1->hashstr, true
grpc_php_wrap_channel_credentials(creds, cred1_hashstr, true
TSRMLS_CC);
RETURN_DESTROY_ZVAL(creds_object);
}

2
src/php/ext/grpc/php7_wrapper.h
Unescape View File

@ -33,6 +33,7 @@
#define php_grpc_add_next_index_stringl(data, str, len, b) \
add_next_index_stringl(data, str, len, b)
#define PHP_GRPC_RETVAL_STRING(val, dup) RETVAL_STRING(val, dup)
#define PHP_GRPC_RETURN_STRING(val, dup) RETURN_STRING(val, dup)
#define PHP_GRPC_MAKE_STD_ZVAL(pzv) MAKE_STD_ZVAL(pzv)
#define PHP_GRPC_FREE_STD_ZVAL(pzv)
@ -145,6 +146,7 @@ static inline int php_grpc_zend_hash_find(HashTable *ht, char *key, int len,
#define php_grpc_add_next_index_stringl(data, str, len, b) \
add_next_index_stringl(data, str, len)
#define PHP_GRPC_RETVAL_STRING(val, dup) RETVAL_STRING(val)
#define PHP_GRPC_RETURN_STRING(val, dup) RETURN_STRING(val)
#define PHP_GRPC_MAKE_STD_ZVAL(pzv) \
pzv = (zval *)emalloc(sizeof(zval));

Write Preview
Loading…
Cancel
Save