diff options
author | Jisi Liu <liujisi@google.com> | 2018-02-26 10:05:09 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-02-26 10:05:09 -0800 |
commit | bb40c0c0c4482cd97e250b24379c1b594ecc0a12 (patch) | |
tree | 139ffeb582df16a54f9f0ab5800a0793ef0e7cf7 /php | |
parent | a6037c5230c0ccb6531f80cef909dbfe37257884 (diff) | |
parent | 91317c291ec819d13c83f924b39f7769fe479673 (diff) | |
download | protobuf-bb40c0c0c4482cd97e250b24379c1b594ecc0a12.tar.gz protobuf-bb40c0c0c4482cd97e250b24379c1b594ecc0a12.tar.bz2 protobuf-bb40c0c0c4482cd97e250b24379c1b594ecc0a12.zip |
Merge pull request #4291 from google/3.5.x
Merge 3.5.x to master
Diffstat (limited to 'php')
-rw-r--r-- | php/ext/google/protobuf/encode_decode.c | 5 | ||||
-rw-r--r-- | php/ext/google/protobuf/map.c | 44 | ||||
-rw-r--r-- | php/ext/google/protobuf/message.c | 59 | ||||
-rw-r--r-- | php/ext/google/protobuf/protobuf.c | 37 | ||||
-rw-r--r-- | php/ext/google/protobuf/protobuf.h | 11 | ||||
-rwxr-xr-x | php/tests/gdb_test.sh | 12 | ||||
-rw-r--r-- | php/tests/memory_leak_test.php | 38 | ||||
-rwxr-xr-x | php/tests/test.sh | 20 |
8 files changed, 177 insertions, 49 deletions
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 24472682..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); } @@ -445,8 +447,7 @@ static void init_file_wrappers(TSRMLS_D); // Define file init functions static void init_file_any(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_any) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0acd010a19676f6f676c652f70726f746f6275662f616e792e70726f746f" @@ -461,12 +462,11 @@ static void init_file_any(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_any = true; } static void init_file_api(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_api) return; init_file_source_context(TSRMLS_C); init_file_type(TSRMLS_C); init_generated_pool_once(TSRMLS_C); @@ -502,12 +502,11 @@ static void init_file_api(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_api = true; } static void init_file_duration(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_duration) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ae3010a1e676f6f676c652f70726f746f6275662f6475726174696f6e2e" @@ -523,12 +522,11 @@ static void init_file_duration(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_duration = true; } static void init_file_field_mask(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_field_mask) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ae3010a20676f6f676c652f70726f746f6275662f6669656c645f6d6173" @@ -544,12 +542,11 @@ static void init_file_field_mask(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_field_mask = true; } static void init_file_empty(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_empty) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ab7010a1b676f6f676c652f70726f746f6275662f656d7074792e70726f" @@ -564,12 +561,11 @@ static void init_file_empty(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_empty = true; } static void init_file_source_context(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_source_context) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0afb010a24676f6f676c652f70726f746f6275662f736f757263655f636f" @@ -586,12 +582,11 @@ static void init_file_source_context(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_source_context = true; } static void init_file_struct(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_struct) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0a81050a1c676f6f676c652f70726f746f6275662f7374727563742e7072" @@ -621,12 +616,11 @@ static void init_file_struct(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_struct = true; } static void init_file_timestamp(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_timestamp) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0ae7010a1f676f6f676c652f70726f746f6275662f74696d657374616d70" @@ -642,12 +636,11 @@ static void init_file_timestamp(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_timestamp = true; } static void init_file_type(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_type) return; init_file_any(TSRMLS_C); init_file_source_context(TSRMLS_C); init_generated_pool_once(TSRMLS_C); @@ -711,12 +704,11 @@ static void init_file_type(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_type = true; } static void init_file_wrappers(TSRMLS_D) { - static bool is_initialized = false; - if (is_initialized) return; + if (is_inited_file_wrappers) return; init_generated_pool_once(TSRMLS_C); const char* generated_file = "0abf030a1e676f6f676c652f70726f746f6275662f77726170706572732e" @@ -739,7 +731,7 @@ static void init_file_wrappers(TSRMLS_D) { hex_to_binary(generated_file, &binary, &binary_len); internal_add_generated_file(binary, binary_len, generated_pool TSRMLS_CC); FREE(binary); - is_initialized = true; + is_inited_file_wrappers = true; } // ----------------------------------------------------------------------------- @@ -928,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. @@ -963,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); @@ -991,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. @@ -1008,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); } @@ -1040,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. diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index daebb460..da00302f 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -145,6 +145,21 @@ PHP_PROTO_HASHTABLE_VALUE get_proto_obj(const char* proto) { } // ----------------------------------------------------------------------------- +// Well Known Types. +// ----------------------------------------------------------------------------- + +bool is_inited_file_any; +bool is_inited_file_api; +bool is_inited_file_duration; +bool is_inited_file_field_mask; +bool is_inited_file_empty; +bool is_inited_file_source_context; +bool is_inited_file_struct; +bool is_inited_file_timestamp; +bool is_inited_file_type; +bool is_inited_file_wrappers; + +// ----------------------------------------------------------------------------- // Reserved Name. // ----------------------------------------------------------------------------- @@ -250,6 +265,17 @@ static PHP_RINIT_FUNCTION(protobuf) { generated_pool_php = NULL; internal_generated_pool_php = NULL; + is_inited_file_any = false; + is_inited_file_api = false; + is_inited_file_duration = false; + is_inited_file_field_mask = false; + is_inited_file_empty = false; + is_inited_file_source_context = false; + is_inited_file_struct = false; + is_inited_file_timestamp = false; + is_inited_file_type = false; + is_inited_file_wrappers = false; + return 0; } @@ -288,6 +314,17 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { } #endif + is_inited_file_any = true; + is_inited_file_api = true; + is_inited_file_duration = true; + is_inited_file_field_mask = true; + is_inited_file_empty = true; + is_inited_file_source_context = true; + is_inited_file_struct = true; + is_inited_file_timestamp = true; + is_inited_file_type = true; + is_inited_file_wrappers = true; + return 0; } diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 6ab0f134..be1de5cf 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -1172,6 +1172,17 @@ extern zend_class_entry* oneof_descriptor_type; // Well Known Type. // ----------------------------------------------------------------------------- +extern bool is_inited_file_any; +extern bool is_inited_file_api; +extern bool is_inited_file_duration; +extern bool is_inited_file_field_mask; +extern bool is_inited_file_empty; +extern bool is_inited_file_source_context; +extern bool is_inited_file_struct; +extern bool is_inited_file_timestamp; +extern bool is_inited_file_type; +extern bool is_inited_file_wrappers; + PHP_METHOD(GPBMetadata_Any, initOnce); PHP_METHOD(GPBMetadata_Api, initOnce); PHP_METHOD(GPBMetadata_Duration, initOnce); diff --git a/php/tests/gdb_test.sh b/php/tests/gdb_test.sh index 484e2edf..a5f6306e 100755 --- a/php/tests/gdb_test.sh +++ b/php/tests/gdb_test.sh @@ -1,10 +1,18 @@ #!/bin/bash +VERSION=$1 + +export PATH=/usr/local/php-$VERSION/bin:$PATH +export C_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$C_INCLUDE_PATH +export CPLUS_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$CPLUS_INCLUDE_PATH + +php -i | grep "Configuration" + # gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which # phpunit` --bootstrap autoload.php tmp_test.php # -gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php +# gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php encode_decode_test.php # -# gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php +gdb --args php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php # # USE_ZEND_ALLOC=0 valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php diff --git a/php/tests/memory_leak_test.php b/php/tests/memory_leak_test.php index 507635e7..ad55d578 100644 --- a/php/tests/memory_leak_test.php +++ b/php/tests/memory_leak_test.php @@ -50,6 +50,13 @@ $to->mergeFromString($data); TestUtil::assertTestMessage($to); +$from = new TestMessage(); +TestUtil::setTestMessage2($from); + +$data = $from->serializeToString(); + +$to->mergeFromString($data); + // TODO(teboring): This causes following tests fail in php7. # $from->setRecursive($from); @@ -104,7 +111,7 @@ assert(1 === $n->getOneofMessage()->getA()); $m = new TestMessage(); $m->mergeFromString(hex2bin('F80601')); -assert('F80601', bin2hex($m->serializeToString())); +assert('f80601' === bin2hex($m->serializeToString())); // Test create repeated field via array. $str_arr = array("abc"); @@ -142,13 +149,32 @@ $from = new \Google\Protobuf\Value(); $from->setNumberValue(1); assert(1, $from->getNumberValue()); +// Test discard unknown in message. +$m = new TestMessage(); +$from = hex2bin('F80601'); +$m->mergeFromString($from); +$m->discardUnknownFields(); +$to = $m->serializeToString(); +assert("" === bin2hex($to)); + +// Test clear +$m = new TestMessage(); +TestUtil::setTestMessage($m); +$m->clear(); + +// Test unset map element +$m = new TestMessage(); +$map = $m->getMapStringString(); +$map[1] = 1; +unset($map[1]); + // Test descriptor $pool = \Google\Protobuf\DescriptorPool::getGeneratedPool(); $desc = $pool->getDescriptorByClassName("\Foo\TestMessage"); $field = $desc->getField(1); -# $from = new TestMessage(); -# $to = new TestMessage(); -# TestUtil::setTestMessage($from); -# $to->mergeFrom($from); -# TestUtil::assertTestMessage($to); +$from = new TestMessage(); +$to = new TestMessage(); +TestUtil::setTestMessage($from); +$to->mergeFrom($from); +TestUtil::assertTestMessage($to); diff --git a/php/tests/test.sh b/php/tests/test.sh index c35372d3..2983fe99 100755 --- a/php/tests/test.sh +++ b/php/tests/test.sh @@ -1,5 +1,11 @@ #!/bin/bash +VERSION=$1 + +export PATH=/usr/local/php-$VERSION/bin:$PATH +export C_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$C_INCLUDE_PATH +export CPLUS_INCLUDE_PATH=/usr/local/php-$VERSION/include/php/main:/usr/local/php-$VERSION/include/php:$CPLUS_INCLUDE_PATH + # Compile c extension pushd ../ext/google/protobuf/ make clean || true @@ -15,7 +21,7 @@ do echo "****************************" echo "* $t" echo "****************************" - php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t + # php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t echo "" done @@ -25,3 +31,15 @@ done export ZEND_DONT_UNLOAD_MODULES=1 export USE_ZEND_ALLOC=0 valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so memory_leak_test.php + +# TODO(teboring): Only for debug (phpunit has memory leak which blocks this beging used by +# regresssion test.) + +# for t in "${tests[@]}" +# do +# echo "****************************" +# echo "* $t (memory leak)" +# echo "****************************" +# valgrind --leak-check=yes php -dextension=../ext/google/protobuf/modules/protobuf.so `which phpunit` --bootstrap autoload.php $t +# echo "" +# done |