From 51293f36d8742b5cc351e5a877928fac77b75322 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Thu, 25 Jan 2018 11:31:05 -0800 Subject: Fix more memory leak for php c extension (#4211) * Fix more memory leak for php c extension * Fix memory leak for php5.5 --- php/ext/google/protobuf/encode_decode.c | 5 ++-- php/ext/google/protobuf/map.c | 44 +++++++++++++++++++++++++++------ php/ext/google/protobuf/message.c | 9 +++++++ 3 files changed, 48 insertions(+), 10 deletions(-) (limited to 'php/ext') diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index b98121bb..a8c47f4d 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -484,11 +484,11 @@ static void map_slot_init(void* memory, upb_fieldtype_t type, zval* cache) { // Store zval** in memory in order to be consistent with the layout of // singular fields. zval** holder = ALLOC(zval*); + *(zval***)memory = holder; zval* tmp; MAKE_STD_ZVAL(tmp); PHP_PROTO_ZVAL_STRINGL(tmp, "", 0, 1); *holder = tmp; - *(zval***)memory = holder; #else *(zval**)memory = cache; PHP_PROTO_ZVAL_STRINGL(*(zval**)memory, "", 0, 1); @@ -521,7 +521,7 @@ static void map_slot_uninit(void* memory, upb_fieldtype_t type) { case UPB_TYPE_BYTES: { #if PHP_MAJOR_VERSION < 7 zval** holder = *(zval***)memory; - php_proto_zval_ptr_dtor(*holder); + zval_ptr_dtor(holder); FREE(holder); #else php_proto_zval_ptr_dtor(*(zval**)memory); @@ -1621,6 +1621,7 @@ static void discard_unknown_fields(MessageHeader* msg) { stringsink* unknown = DEREF(message_data(msg), 0, stringsink*); if (unknown != NULL) { stringsink_uninit(unknown); + FREE(unknown); DEREF(message_data(msg), 0, stringsink*) = NULL; } diff --git a/php/ext/google/protobuf/map.c b/php/ext/google/protobuf/map.c index 2680b547..ab8a518a 100644 --- a/php/ext/google/protobuf/map.c +++ b/php/ext/google/protobuf/map.c @@ -293,13 +293,46 @@ static bool map_field_read_dimension(zval *object, zval *key, int type, } } +static bool map_index_unset(Map *intern, const char* keyval, int length) { + upb_value old_value; + if (upb_strtable_remove2(&intern->table, keyval, length, &old_value)) { + switch (intern->value_type) { + case UPB_TYPE_MESSAGE: { +#if PHP_MAJOR_VERSION < 7 + zval_ptr_dtor(upb_value_memory(&old_value)); +#else + zend_object* object = *(zend_object**)upb_value_memory(&old_value); + if(--GC_REFCOUNT(object) == 0) { + zend_objects_store_del(object); + } +#endif + break; + } + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: { +#if PHP_MAJOR_VERSION < 7 + zval_ptr_dtor(upb_value_memory(&old_value)); +#else + zend_string* object = *(zend_string**)upb_value_memory(&old_value); + zend_string_release(object); +#endif + break; + } + default: + break; + } + } +} + bool map_index_set(Map *intern, const char* keyval, int length, upb_value v) { // Replace any existing value by issuing a 'remove' operation first. - upb_strtable_remove2(&intern->table, keyval, length, NULL); + map_index_unset(intern, keyval, length); + if (!upb_strtable_insert2(&intern->table, keyval, length, v)) { zend_error(E_USER_ERROR, "Could not insert into table"); return false; } + return true; } @@ -326,12 +359,7 @@ static void map_field_write_dimension(zval *object, zval *key, v.ctype = UPB_CTYPE_UINT64; #endif - // Replace any existing value by issuing a 'remove' operation first. - upb_strtable_remove2(&intern->table, keyval, length, NULL); - if (!upb_strtable_insert2(&intern->table, keyval, length, v)) { - zend_error(E_USER_ERROR, "Could not insert into table"); - return; - } + map_index_set(intern, keyval, length, v); } static bool map_field_unset_dimension(zval *object, zval *key TSRMLS_DC) { @@ -348,7 +376,7 @@ static bool map_field_unset_dimension(zval *object, zval *key TSRMLS_DC) { v.ctype = UPB_CTYPE_UINT64; #endif - upb_strtable_remove2(&intern->table, keyval, length, &v); + map_index_unset(intern, keyval, length); return true; } diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 60f2225e..e801f4a0 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -292,7 +292,9 @@ PHP_METHOD(Message, clear) { Descriptor* desc = msg->descriptor; zend_class_entry* ce = desc->klass; + zend_object_std_dtor(&msg->std TSRMLS_CC); object_properties_init(&msg->std, ce); + layout_init(desc->layout, message_data(msg), &msg->std TSRMLS_CC); } @@ -918,6 +920,7 @@ PHP_METHOD(Any, unpack) { PHP_PROTO_FAKE_SCOPE_BEGIN(any_type); zval* type_url_php = php_proto_message_read_property( getThis(), &type_url_member PHP_PROTO_TSRMLS_CC); + zval_dtor(&type_url_member); PHP_PROTO_FAKE_SCOPE_END; // Get fully-qualified name from type url. @@ -953,6 +956,7 @@ PHP_METHOD(Any, unpack) { PHP_PROTO_FAKE_SCOPE_RESTART(any_type); zval* value = php_proto_message_read_property( getThis(), &value_member PHP_PROTO_TSRMLS_CC); + zval_dtor(&value_member); PHP_PROTO_FAKE_SCOPE_END; merge_from_string(Z_STRVAL_P(value), Z_STRLEN_P(value), desc, msg); @@ -981,6 +985,8 @@ PHP_METHOD(Any, pack) { PHP_PROTO_FAKE_SCOPE_BEGIN(any_type); message_handlers->write_property(getThis(), &member, &data, NULL PHP_PROTO_TSRMLS_CC); + zval_dtor(&data); + zval_dtor(&member); PHP_PROTO_FAKE_SCOPE_END; // Set type url. @@ -998,6 +1004,8 @@ PHP_METHOD(Any, pack) { PHP_PROTO_FAKE_SCOPE_RESTART(any_type); message_handlers->write_property(getThis(), &member, &type_url_php, NULL PHP_PROTO_TSRMLS_CC); + zval_dtor(&type_url_php); + zval_dtor(&member); PHP_PROTO_FAKE_SCOPE_END; FREE(type_url); } @@ -1030,6 +1038,7 @@ PHP_METHOD(Any, is) { PHP_PROTO_FAKE_SCOPE_BEGIN(any_type); zval* value = php_proto_message_read_property(getThis(), &member PHP_PROTO_TSRMLS_CC); + zval_dtor(&member); PHP_PROTO_FAKE_SCOPE_END; // Compare two type url. -- cgit v1.2.3