diff --git a/NEWS b/NEWS index 12740c65c3c89..c06519d86e7e0 100644 --- a/NEWS +++ b/NEWS @@ -64,6 +64,7 @@ PHP NEWS - SPL: . DirectoryIterator key can now work better with filesystem supporting larger directory indexing. (David Carlier) + . Fixed bug GH-17935 (Add delayed separation in ArrayObject). (alexandre-daubois) - Sqlite3: . Fix NUL byte truncation in sqlite3 TEXT column handling. (ndossche) diff --git a/UPGRADING b/UPGRADING index d52827bf96154..fa7d304f3ed55 100644 --- a/UPGRADING +++ b/UPGRADING @@ -23,6 +23,10 @@ PHP 8.6 UPGRADE NOTES . Invalid values now throw in Phar::mungServer() instead of being silently ignored. +- SPL: + . RecursiveArrayIterator modifications now follow standard CoW semantics and + child modifications no longer affect the parent array. + ======================================== 2. New Features ======================================== diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index 3d6870a7ee953..ae61129d2c3a9 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -44,8 +44,6 @@ typedef struct _spl_array_object { uint32_t ht_iter; int ar_flags; unsigned char nApplyCount; - bool is_child; - Bucket *bucket; zend_function *fptr_offset_get; zend_function *fptr_offset_set; zend_function *fptr_offset_has; @@ -166,8 +164,6 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o object_properties_init(&intern->std, class_type); intern->ar_flags = 0; - intern->is_child = false; - intern->bucket = NULL; intern->ce_get_iterator = spl_ce_ArrayIterator; if (orig) { spl_array_object *other = spl_array_from_obj(orig); @@ -466,21 +462,12 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ } /* }}} */ /* - * The assertion(HT_ASSERT_RC1(ht)) failed because the refcount was increased manually when intern->is_child is true. - * We have to set the refcount to 1 to make assertion success and restore the refcount to the original value after - * modifying the array when intern->is_child is true. + * Before modifying the internal array, we must ensure exclusive ownership + * to satisfy HT_ASSERT_RC1 in Zend hash operations. + * + * SEPARATE_ARRAY implements CoW: if the array is shared (RC > 1), + * it creates a duplicate via zend_array_dup() and updates intern->array. */ -static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t refcount) /* {{{ */ -{ - uint32_t old_refcount = 0; - if (is_child) { - old_refcount = GC_REFCOUNT(ht); - GC_SET_REFCOUNT(ht, refcount); - } - - return old_refcount; -} /* }}} */ - static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */ { spl_array_object *intern = spl_array_from_obj(object); @@ -505,18 +492,15 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec Z_TRY_ADDREF_P(value); - uint32_t refcount = 0; if (!offset || Z_TYPE_P(offset) == IS_NULL) { + if (Z_TYPE(intern->array) == IS_ARRAY) { + SEPARATE_ARRAY(&intern->array); + } ht = spl_array_get_hash_table(intern); if (UNEXPECTED(ht == intern->sentinel_array)) { return; } - refcount = spl_array_set_refcount(intern->is_child, ht, 1); zend_hash_next_index_insert(ht, value); - - if (refcount) { - spl_array_set_refcount(intern->is_child, ht, refcount); - } return; } @@ -526,22 +510,20 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec return; } + if (Z_TYPE(intern->array) == IS_ARRAY) { + SEPARATE_ARRAY(&intern->array); + } ht = spl_array_get_hash_table(intern); if (UNEXPECTED(ht == intern->sentinel_array)) { spl_hash_key_release(&key); return; } - refcount = spl_array_set_refcount(intern->is_child, ht, 1); if (key.key) { zend_hash_update_ind(ht, key.key, value); spl_hash_key_release(&key); } else { zend_hash_index_update(ht, key.h, value); } - - if (refcount) { - spl_array_set_refcount(intern->is_child, ht, refcount); - } } /* }}} */ static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */ @@ -570,8 +552,10 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec return; } + if (Z_TYPE(intern->array) == IS_ARRAY) { + SEPARATE_ARRAY(&intern->array); + } ht = spl_array_get_hash_table(intern); - uint32_t refcount = spl_array_set_refcount(intern->is_child, ht, 1); if (key.key) { zval *data = zend_hash_find(ht, key.key); @@ -597,10 +581,6 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec } else { zend_hash_index_del(ht, key.h); } - - if (refcount) { - spl_array_set_refcount(intern->is_child, ht, refcount); - } } /* }}} */ static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{ */ @@ -970,17 +950,7 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar if (Z_REFCOUNT_P(array) == 1) { ZVAL_COPY(&intern->array, array); } else { - //??? TODO: try to avoid array duplication ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array))); - - if (intern->is_child) { - Z_TRY_DELREF(intern->bucket->val); - /* - * replace bucket->val with copied array, so the changes between - * parent and child object can affect each other. - */ - ZVAL_COPY(&intern->bucket->val, &intern->array); - } } } else { php_error_docref(NULL, E_DEPRECATED, @@ -1850,15 +1820,6 @@ PHP_METHOD(RecursiveArrayIterator, hasChildren) static void spl_instantiate_child_arg(zend_class_entry *pce, zval *retval, zval *arg1, zval *arg2) /* {{{ */ { object_init_ex(retval, pce); - spl_array_object *new_intern = Z_SPLARRAY_P(retval); - /* - * set new_intern->is_child is true to indicate that the object was created by - * RecursiveArrayIterator::getChildren() method. - */ - new_intern->is_child = true; - - /* find the bucket of parent object. */ - new_intern->bucket = (Bucket *)((char *)(arg1) - XtOffsetOf(Bucket, val));; zend_call_known_instance_method_with_2_params(pce->constructor, Z_OBJ_P(retval), NULL, arg1, arg2); } /* }}} */ diff --git a/ext/spl/tests/ArrayObject/gh10519.phpt b/ext/spl/tests/ArrayObject/gh10519.phpt index 5ef7fb3144f92..d07c3a2a34bd8 100644 --- a/ext/spl/tests/ArrayObject/gh10519.phpt +++ b/ext/spl/tests/ArrayObject/gh10519.phpt @@ -51,6 +51,7 @@ $example->bugySetMethod(5, 'in here'); var_dump(json_encode($example)); var_dump(json_encode($a)); +// child iterator modifications do not propagate to the parent iterator when working with arrays $b = [ 'test' => [ 'b' => [2 => '',3 => '',4 => ''], @@ -66,5 +67,5 @@ var_dump(json_encode($b)); Deprecated: ArrayIterator::__construct(): Using an object as a backing array for ArrayIterator is deprecated, as it allows violating class constraints and invariants in %s on line %d string(51) "{"test":{"a":{"2":"","3":"","4":"","5":"in here"}}}" string(51) "{"test":{"a":{"2":"","3":"","4":"","5":"in here"}}}" -string(56) "{"test":{"b":{"2":"","3":"","4":"","5":"must be here"}}}" +string(37) "{"test":{"b":{"2":"","3":"","4":""}}}" string(37) "{"test":{"b":{"2":"","3":"","4":""}}}" diff --git a/ext/spl/tests/bug42654.phpt b/ext/spl/tests/bug42654.phpt index eb72863e08759..20aad74b73423 100644 --- a/ext/spl/tests/bug42654.phpt +++ b/ext/spl/tests/bug42654.phpt @@ -110,11 +110,11 @@ object(RecursiveArrayIterator)#%d (1) { [2]=> array(2) { [2]=> - string(5) "alter" + string(4) "val2" [3]=> array(1) { [3]=> - string(5) "alter" + string(4) "val3" } } [4]=> @@ -129,11 +129,11 @@ object(RecursiveArrayIterator)#%d (1) { [2]=> array(2) { [2]=> - string(5) "alter" + string(4) "val2" [3]=> array(1) { [3]=> - string(5) "alter" + string(4) "val3" } } [4]=> @@ -146,11 +146,11 @@ array(3) { [2]=> array(2) { [2]=> - string(5) "alter" + string(4) "val2" [3]=> array(1) { [3]=> - string(5) "alter" + string(4) "val3" } } [4]=> diff --git a/ext/spl/tests/bug42654_2.phpt b/ext/spl/tests/bug42654_2.phpt index bd290247dbdc4..431920e1439b2 100644 --- a/ext/spl/tests/bug42654_2.phpt +++ b/ext/spl/tests/bug42654_2.phpt @@ -26,7 +26,7 @@ array(3) { [0]=> array(1) { ["key2"]=> - string(5) "alter" + string(4) "val2" } ["key3"]=> string(4) "val3" diff --git a/ext/spl/tests/gh17935.phpt b/ext/spl/tests/gh17935.phpt new file mode 100644 index 0000000000000..c15adbfa4104d --- /dev/null +++ b/ext/spl/tests/gh17935.phpt @@ -0,0 +1,11 @@ +--TEST-- +GH-17935 (ArrayObject __serialize() should not cause assertion failure on modification) +--FILE-- +__serialize(); +$o['a'] = 'b'; +echo "Success\n"; +?> +--EXPECT-- +Success