HashTableIterators of empty hashtables are not poisoned upon destruction

[bwoebi]

Description

Using the zend_hash_iterators_* API, non-empty arrays are properly marked as poisoned in the HashTableIterator entries when destroyed.

However there is an edge case: if nNumUsed == 0 (thanks to deletion of entries during iteration), then the zend_hash_iterators_remove call is skipped in zend_hash_destroy.

Thus, we either should:
a) fix this by always calling the removal ... or
b) completely remove the auto-removal on destruction and leave it the extension authors responsibility to clear the iterators (making zend_hash_iterators_remove public API)

From the perspective of the code in php-src there is no need to have the auto-removal on free, as every iterating thing holds a reference to it.

PHP Version

master

Operating System

No response

[nikic]

b) completely remove the auto-removal on destruction and leave it the extension authors responsibility to clear the iterators (making zend_hash_iterators_remove public API)

I don't think that would work for the primary consumer (foreach), because the array may be destroyed outside its control (reassignment of the reference).

[bwoebi]

As it is right now, we always seem to route user-controlled arrays through zend_array_destroy which doesn't expose this problem. zend_array_destroy always properly releases its iterators. zend_hash_destroy does not, if nNumUsed == 0.

foreach would be only impacted if we were to remove it from zend_array_destroy too.

[nikic]

Seems pretty clear that zend_hash_destroy should be fixed then, if the behavior is not consistent with zend_array_destroy?