From 7fcce35f9b53e4970b26677433aaedd3098d13c9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Mon, 25 Oct 2010 01:41:54 +0000 Subject: [PATCH] - Fixed bug #53071 (SPLObjectStorage defeats gc_collect_cycles). --- ext/spl/spl_observer.c | 65 +++++++++++++++++++++++++++++++++++-- ext/spl/tests/bug53071.phpt | 26 +++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 ext/spl/tests/bug53071.phpt diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c index 2ba80552dd..2f3a29f441 100755 --- a/ext/spl/spl_observer.c +++ b/ext/spl/spl_observer.c @@ -324,6 +324,8 @@ static HashTable* spl_object_storage_debug_info(zval *obj, int *is_temp TSRMLS_D *is_temp = 0; props = Z_OBJPROP_P(obj); + zend_hash_del(props, "\x00gcdata", sizeof("\x00gcdata")); + if (intern->debug_info == NULL) { ALLOC_HASHTABLE(intern->debug_info); ZEND_INIT_SYMTABLE_EX(intern->debug_info, zend_hash_num_elements(props) + 1, 0); @@ -338,10 +340,11 @@ static HashTable* spl_object_storage_debug_info(zval *obj, int *is_temp TSRMLS_D zend_hash_internal_pointer_reset_ex(&intern->storage, &pos); while (zend_hash_get_current_data_ex(&intern->storage, (void **)&element, &pos) == SUCCESS) { php_spl_object_hash(element->obj, md5str TSRMLS_CC); - Z_ADDREF_P(element->obj); - Z_ADDREF_P(element->inf); MAKE_STD_ZVAL(tmp); array_init(tmp); + /* Incrementing the refcount of obj and inf would confuse the garbage collector. + * Prefer to null the destructor */ + Z_ARRVAL_P(tmp)->pDestructor = NULL; add_assoc_zval_ex(tmp, "obj", sizeof("obj"), element->obj); add_assoc_zval_ex(tmp, "inf", sizeof("inf"), element->inf); add_assoc_zval_ex(storage, md5str, 33, tmp); @@ -357,6 +360,63 @@ static HashTable* spl_object_storage_debug_info(zval *obj, int *is_temp TSRMLS_D } /* }}} */ +/* overriden for garbage collection + * This is very hacky, but unfortunately the garbage collector can only query objects for + * dependencies through get_properties */ +static HashTable *spl_object_storage_get_properties(zval *obj TSRMLS_DC) /* {{{ */ +{ + spl_SplObjectStorage *intern = (spl_SplObjectStorage*)zend_object_store_get_object(obj TSRMLS_CC); + spl_SplObjectStorageElement *element; + HashTable *props; + HashPosition pos; + zval *gcdata_arr = NULL, + **gcdata_arr_pp; + + props = std_object_handlers.get_properties(obj TSRMLS_CC); + + if (!GC_G(gc_active)) { + zend_hash_del(props, "\x00gcdata", sizeof("\x00gcdata")); + return props; + } + + if (props->nApplyCount > 0) { + return props; + } + + /* clean \x00gcdata, as it may be out of date */ + if (zend_hash_find(props, "\x00gcdata", sizeof("\x00gcdata"), (void**) &gcdata_arr_pp) == SUCCESS) { + gcdata_arr = *gcdata_arr_pp; + zend_hash_clean(Z_ARRVAL_P(gcdata_arr)); + } + + /* destroy intern->debug_info, as it's holding references to the zvals */ + if (intern->debug_info != NULL) { + zend_hash_destroy(intern->debug_info); + efree(intern->debug_info); + intern->debug_info = NULL; + } + + if (gcdata_arr == NULL) { + MAKE_STD_ZVAL(gcdata_arr); + array_init(gcdata_arr); + /* don't decrease refcount of members when destroying */ + Z_ARRVAL_P(gcdata_arr)->pDestructor = NULL; + + /* name starts with \x00 to make tampering in user-land more difficult */ + zend_hash_add(props, "\x00gcdata", sizeof("\x00gcdata"), &gcdata_arr, sizeof(gcdata_arr), NULL); + } + + zend_hash_internal_pointer_reset_ex(&intern->storage, &pos); + while (zend_hash_get_current_data_ex(&intern->storage, (void **)&element, &pos) == SUCCESS) { + add_next_index_zval(gcdata_arr, element->obj); + add_next_index_zval(gcdata_arr, element->inf); + zend_hash_move_forward_ex(&intern->storage, &pos); + } + + return props; +} +/* }}} */ + static int spl_object_storage_compare_info(spl_SplObjectStorageElement *e1, spl_SplObjectStorageElement *e2 TSRMLS_DC) /* {{{ */ { zval result; @@ -1178,6 +1238,7 @@ PHP_MINIT_FUNCTION(spl_observer) REGISTER_SPL_STD_CLASS_EX(SplObjectStorage, spl_SplObjectStorage_new, spl_funcs_SplObjectStorage); memcpy(&spl_handler_SplObjectStorage, zend_get_std_object_handlers(), sizeof(zend_object_handlers)); + spl_handler_SplObjectStorage.get_properties = spl_object_storage_get_properties; spl_handler_SplObjectStorage.get_debug_info = spl_object_storage_debug_info; spl_handler_SplObjectStorage.compare_objects = spl_object_storage_compare_objects; spl_handler_SplObjectStorage.clone_obj = spl_object_storage_clone; diff --git a/ext/spl/tests/bug53071.phpt b/ext/spl/tests/bug53071.phpt new file mode 100644 index 0000000000..b0ea3aad84 --- /dev/null +++ b/ext/spl/tests/bug53071.phpt @@ -0,0 +1,26 @@ +--TEST-- +Bug #53071 (Usage of SPLObjectStorage defeats gc_collect_cycles) +--FILE-- +member = $myA; // myC has a referece to myA + $myB->Attach($myC); // myB attaches myC + $myA->member = $myB; // myA has myB, comleting the cycle +} +LimitedScope(); +var_dump(gc_collect_cycles()); + +echo "Done.\n"; + +?> +--EXPECTF-- +int(5) +Done. -- 2.40.0