diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c index 622686dbd78a6..e653d1173b246 100644 --- a/ext/spl/spl_observer.c +++ b/ext/spl/spl_observer.c @@ -240,11 +240,25 @@ static zend_result spl_object_storage_detach(spl_SplObjectStorage *intern, zend_ return ret; } /* }}}*/ +/* TODO: make this an official Zend API? */ +#define SPL_SAFE_HASH_FOREACH_PTR(_ht, _ptr) do { \ + const HashTable *__ht = (_ht); \ + zval *_z = __ht->arPacked; \ + for (uint32_t _idx = 0; _idx < __ht->nNumUsed; _idx++, _z = ZEND_HASH_ELEMENT(__ht, _idx)) { \ + if (UNEXPECTED(Z_ISUNDEF_P(_z))) continue; \ + _ptr = Z_PTR_P(_z); + static void spl_object_storage_addall(spl_SplObjectStorage *intern, spl_SplObjectStorage *other) { /* {{{ */ spl_SplObjectStorageElement *element; - ZEND_HASH_FOREACH_PTR(&other->storage, element) { - spl_object_storage_attach(intern, element->obj, &element->inf); + SPL_SAFE_HASH_FOREACH_PTR(&other->storage, element) { + zval zv; + zend_object *obj = element->obj; + GC_ADDREF(obj); + ZVAL_COPY(&zv, &element->inf); + spl_object_storage_attach(intern, obj, &zv); + zval_ptr_dtor(&zv); + OBJ_RELEASE(obj); } ZEND_HASH_FOREACH_END(); intern->index = 0; @@ -626,10 +640,13 @@ PHP_METHOD(SplObjectStorage, removeAllExcept) other = Z_SPLOBJSTORAGE_P(obj); - ZEND_HASH_FOREACH_PTR(&intern->storage, element) { - if (!spl_object_storage_contains(other, element->obj)) { - spl_object_storage_detach(intern, element->obj); + SPL_SAFE_HASH_FOREACH_PTR(&intern->storage, element) { + zend_object *elem_obj = element->obj; + GC_ADDREF(elem_obj); + if (!spl_object_storage_contains(other, elem_obj)) { + spl_object_storage_detach(intern, elem_obj); } + OBJ_RELEASE(elem_obj); } ZEND_HASH_FOREACH_END(); zend_hash_internal_pointer_reset_ex(&intern->storage, &intern->pos); diff --git a/ext/spl/tests/SplObjectStorage/concurrent_deletion.phpt b/ext/spl/tests/SplObjectStorage/concurrent_deletion.phpt new file mode 100644 index 0000000000000..ae063f1c9b03a --- /dev/null +++ b/ext/spl/tests/SplObjectStorage/concurrent_deletion.phpt @@ -0,0 +1,46 @@ +--TEST-- +SplObjectStorage: Concurrent deletion during iteration +--CREDITS-- +cnitlrt +--FILE-- +removeAllExcept($other)); + +unset($victim, $other); +$victim = new SplObjectStorage(); +$other = new EvilStorage(); + +for ($i = 0; $i < 1024; $i++) { + $o = new stdClass(); + $victim[$o] = null; + $other[$o] = null; +} + +var_dump($other->addAll($victim)); +?> +--EXPECTF-- +int(%d) +int(1024) diff --git a/ext/spl/tests/SplObjectStorage/concurrent_deletion_addall.phpt b/ext/spl/tests/SplObjectStorage/concurrent_deletion_addall.phpt new file mode 100644 index 0000000000000..af3fc381b5622 --- /dev/null +++ b/ext/spl/tests/SplObjectStorage/concurrent_deletion_addall.phpt @@ -0,0 +1,43 @@ +--TEST-- +SplObjectStorage: Concurrent deletion during addAll +--CREDITS-- +cnitlrt +--FILE-- +addAll($storage); + +var_dump($evil, $storage); + +?> +--EXPECTF-- +object(EvilStorage)#%d (1) { + ["storage":"SplObjectStorage":private]=> + array(1) { + [0]=> + array(2) { + ["obj"]=> + object(stdClass)#%d (0) { + } + ["inf"]=> + string(3) "foo" + } + } +} +object(SplObjectStorage)#%d (1) { + ["storage":"SplObjectStorage":private]=> + array(0) { + } +} diff --git a/ext/spl/tests/SplObjectStorage/concurrent_deletion_removeexcept.phpt b/ext/spl/tests/SplObjectStorage/concurrent_deletion_removeexcept.phpt new file mode 100644 index 0000000000000..b2ed211b304af --- /dev/null +++ b/ext/spl/tests/SplObjectStorage/concurrent_deletion_removeexcept.phpt @@ -0,0 +1,35 @@ +--TEST-- +SplObjectStorage: Concurrent deletion during removeAllExcept +--CREDITS-- +cnitlrt +--FILE-- +removeAllExcept($evil); + +var_dump($evil, $storage); + +?> +--EXPECTF-- +object(EvilStorage)#%d (1) { + ["storage":"SplObjectStorage":private]=> + array(0) { + } +} +object(SplObjectStorage)#%d (1) { + ["storage":"SplObjectStorage":private]=> + array(0) { + } +}