diff --git a/README.md b/README.md index afcfd44..00a7ccb 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,8 @@ Documentation is available on [php.net](https://www.php.net/manual/en/book.ds.ph - **Ds\Key** -- interface for custom key equality (replaces Hashable) `getIterator()` on `Seq`, `Map`, `Set`, and `Heap` returns an `InternalIterator` supporting the standard `Iterator` methods. +Heap iterators retain a snapshot of the values present when created; `rewind()` restarts that snapshot even if the original heap has changed. +Calling `next()` or `rewind()` on the same iterator from its comparator throws a `RuntimeException`. You should also include the [polyfill](https://github.com/php-ds/polyfill) in your project for IDE integration. diff --git a/src/php/iterators/php_heap_iterator.c b/src/php/iterators/php_heap_iterator.c index 7765ef3..01d4cae 100644 --- a/src/php/iterators/php_heap_iterator.c +++ b/src/php/iterators/php_heap_iterator.c @@ -8,8 +8,11 @@ static void php_ds_heap_iterator_dtor(zend_object_iterator *iter) { php_ds_heap_iterator_t *iterator = (php_ds_heap_iterator_t *) iter; - ds_heap_free(iterator->heap); + if (iterator->heap) { + ds_heap_free(iterator->heap); + } DTOR_AND_UNDEF(&iterator->intern.data); + ds_heap_release(iterator->snapshot); OBJ_RELEASE(iterator->object); } @@ -20,36 +23,61 @@ static int php_ds_heap_iterator_valid(zend_object_iterator *iter) static zval *php_ds_heap_iterator_get_current_data(zend_object_iterator *iter) { - return &iter->data; + return Z_ISUNDEF(iter->data) ? NULL : &iter->data; } static void php_ds_heap_iterator_get_current_key(zend_object_iterator *iter, zval *key) { - ZVAL_LONG(key, ((php_ds_heap_iterator_t *) iter)->position); + if (Z_ISUNDEF(iter->data)) { + ZVAL_NULL(key); + } else { + ZVAL_LONG(key, ((php_ds_heap_iterator_t *) iter)->position); + } } static void php_ds_heap_iterator_set_current(php_ds_heap_iterator_t *iterator) { - if (DS_HEAP_IS_EMPTY(iterator->heap)) { - ZVAL_UNDEF(&iterator->intern.data); - } else { + DTOR_AND_UNDEF(&iterator->intern.data); + + if (!DS_HEAP_IS_EMPTY(iterator->heap)) { ds_heap_pop(iterator->heap, &iterator->intern.data, iterator->compare); - Z_TRY_DELREF_P(&iterator->intern.data); } } static void php_ds_heap_iterator_move_forward(zend_object_iterator *iter) { php_ds_heap_iterator_t *iterator = (php_ds_heap_iterator_t *) iter; + + if (iterator->running) { + ds_throw_exception(spl_ce_RuntimeException, + "Cannot modify a heap iterator while it is executing"); + return; + } + + iterator->running = true; php_ds_heap_iterator_set_current(iterator); iterator->position++; + iterator->running = false; } static void php_ds_heap_iterator_rewind(zend_object_iterator *iter) { php_ds_heap_iterator_t *iterator = (php_ds_heap_iterator_t *) iter; + + if (iterator->running) { + ds_throw_exception(spl_ce_RuntimeException, + "Cannot modify a heap iterator while it is executing"); + return; + } + + iterator->running = true; + if (iterator->heap) { + ds_heap_free(iterator->heap); + } + iterator->heap = ds_heap_clone(iterator->snapshot); php_ds_heap_iterator_set_current(iterator); iterator->position = 0; + iterator->running = false; } static zend_object_iterator_funcs iterator_funcs = { @@ -79,7 +107,8 @@ zend_object_iterator *php_ds_heap_get_iterator(zend_class_entry *ce, zval *objec ZVAL_UNDEF(&iterator->intern.data); iterator->intern.funcs = &iterator_funcs; - iterator->heap = ds_heap_clone(obj->heap); + iterator->snapshot = obj->heap; + iterator->snapshot->refs++; iterator->compare = php_ds_heap_get_compare(obj); iterator->object = Z_OBJ_P(object); iterator->position = 0; diff --git a/src/php/iterators/php_heap_iterator.h b/src/php/iterators/php_heap_iterator.h index 067c36e..5c33710 100644 --- a/src/php/iterators/php_heap_iterator.h +++ b/src/php/iterators/php_heap_iterator.h @@ -7,9 +7,11 @@ typedef struct _php_ds_heap_iterator_t { zend_object_iterator intern; zend_object *object; + ds_heap_t *snapshot; // Shared heap retained for rewinding. ds_heap_t *heap; // Cloned heap — popped during iteration. ds_heap_compare_func_t compare; zend_long position; + bool running; // Prevent modification from comparator callbacks. } php_ds_heap_iterator_t; zend_object_iterator *php_ds_heap_get_iterator(zend_class_entry *ce, zval *obj, int by_ref); diff --git a/tests/Heap/getIterator_current.phpt b/tests/Heap/getIterator_current.phpt new file mode 100644 index 0000000..96f750e --- /dev/null +++ b/tests/Heap/getIterator_current.phpt @@ -0,0 +1,57 @@ +--TEST-- +Heap: iterator owns its current value after the source heap is cleared +--FILE-- + 1]]); +$iterator = $heap->getIterator(); +$heap->clear(); +unset($heap); + +// Releasing a current() result must not free the iterator's current value. +echo json_encode($iterator->current()), "\n"; +echo json_encode($iterator->current()), "\n"; + +// A returned value must also survive destruction of the iterator. +$value = $iterator->current(); +$reference = WeakReference::create($value); +unset($iterator); +echo json_encode($value), "\n"; +unset($value); +var_dump($reference->get()); + +// Advancing and destroying the iterator must release all owned values. +$values = [(object) ['value' => 1], (object) ['value' => 2]]; +$references = array_map(WeakReference::create(...), $values); +$heap = new Ds\Heap($values); +$iterator = $heap->getIterator(); +unset($values); +$heap->clear(); +while ($iterator->valid()) { + echo json_encode($iterator->current()), "\n"; + $iterator->next(); +} +var_dump($iterator->current(), $iterator->key()); +unset($iterator); +foreach ($references as $reference) { + var_dump($reference->get()); +} + +// An iterator can be destroyed before it has ever been rewound. +$heap = new Ds\Heap([new stdClass()]); +$reference = WeakReference::create($heap->peek()); +$iterator = $heap->getIterator(); +unset($heap, $iterator); +var_dump($reference->get()); +?> +--EXPECT-- +{"value":1} +{"value":1} +{"value":1} +NULL +{"value":2} +{"value":1} +NULL +NULL +NULL +NULL +NULL diff --git a/tests/Heap/getIterator_reentrant.phpt b/tests/Heap/getIterator_reentrant.phpt new file mode 100644 index 0000000..be40bf9 --- /dev/null +++ b/tests/Heap/getIterator_reentrant.phpt @@ -0,0 +1,99 @@ +--TEST-- +Heap: comparator callbacks cannot advance or rewind the same iterator +--FILE-- + $inner, caught ", $catchInside ? 'inside' : 'outside', "\n"; + $iterator = null; + $attempt = false; + $heap = new Ds\Heap([1, 2, 3, 4, 5], static function ($a, $b) use ( + &$iterator, &$attempt, $inner, $catchInside + ) { + if ($attempt) { + $attempt = false; + if ($catchInside) { + try { + $iterator->$inner(); + } catch (RuntimeException $e) { + echo $e->getMessage(), "\n"; + } + } else { + $iterator->$inner(); + } + } + return $a <=> $b; + }); + $iterator = $heap->getIterator(); + if ($outer === 'next') { + $iterator->rewind(); + } + + $attempt = true; + try { + $iterator->$outer(); + } catch (RuntimeException $e) { + echo $e->getMessage(), "\n"; + } + + if ($catchInside) { + // A rejected nested operation must not change the key or skip a value. + echo $iterator->key(), ':', $iterator->current(), "\n"; + $iterator->next(); + echo $iterator->key(), ':', $iterator->current(), "\n"; + } + + // The guard must be released even when the comparator propagates an exception. + echo json_encode(iterator_to_array($iterator)), "\n"; + echo json_encode($heap->toArray()), "\n"; + $iterator = null; // Break the reference held by the comparator. +} + +foreach (['rewind', 'next'] as $outer) { + foreach (['rewind', 'next'] as $inner) { + checkReentry($outer, $inner, true); + checkReentry($outer, $inner, false); + } +} +?> +--EXPECT-- +rewind -> rewind, caught inside +Cannot modify a heap iterator while it is executing +0:5 +1:4 +[5,4,3,2,1] +[5,4,3,2,1] +rewind -> rewind, caught outside +Cannot modify a heap iterator while it is executing +[5,4,3,2,1] +[5,4,3,2,1] +rewind -> next, caught inside +Cannot modify a heap iterator while it is executing +0:5 +1:4 +[5,4,3,2,1] +[5,4,3,2,1] +rewind -> next, caught outside +Cannot modify a heap iterator while it is executing +[5,4,3,2,1] +[5,4,3,2,1] +next -> rewind, caught inside +Cannot modify a heap iterator while it is executing +1:4 +2:3 +[5,4,3,2,1] +[5,4,3,2,1] +next -> rewind, caught outside +Cannot modify a heap iterator while it is executing +[5,4,3,2,1] +[5,4,3,2,1] +next -> next, caught inside +Cannot modify a heap iterator while it is executing +1:4 +2:3 +[5,4,3,2,1] +[5,4,3,2,1] +next -> next, caught outside +Cannot modify a heap iterator while it is executing +[5,4,3,2,1] +[5,4,3,2,1] diff --git a/tests/Heap/getIterator_rewind.phpt b/tests/Heap/getIterator_rewind.phpt new file mode 100644 index 0000000..336a63b --- /dev/null +++ b/tests/Heap/getIterator_rewind.phpt @@ -0,0 +1,50 @@ +--TEST-- +Heap: iterator rewind restores the original snapshot +--FILE-- +getIterator(); + +// Mutation before the first rewind must not change the iterator's snapshot. +$heap->clear(); +$heap->push(99); + +$iterator->rewind(); +echo $iterator->key(), ':', $iterator->current(), "\n"; +$iterator->rewind(); +echo $iterator->key(), ':', $iterator->current(), "\n"; +$iterator->next(); +echo $iterator->key(), ':', $iterator->current(), "\n"; +$iterator->rewind(); +echo $iterator->key(), ':', $iterator->current(), "\n"; + +// foreach rewinds the same iterator, including after it has been exhausted. +echo json_encode(iterator_to_array($iterator)), "\n"; +echo json_encode(iterator_to_array($iterator)), "\n"; +echo json_encode($heap->toArray()), "\n"; + +// Rewinding must preserve the custom comparator's ordering too. +$heap = new Ds\Heap([3, 1, 2], fn($a, $b) => $b <=> $a); +$iterator = $heap->getIterator(); +echo json_encode(iterator_to_array($iterator)), "\n"; +echo json_encode(iterator_to_array($iterator)), "\n"; + +$iterator = (new Ds\Heap())->getIterator(); +$iterator->rewind(); +$iterator->rewind(); +var_dump($iterator->valid()); +var_dump($iterator->current(), $iterator->key()); +?> +--EXPECT-- +0:3 +0:3 +1:2 +0:3 +[3,2,1] +[3,2,1] +[99] +[1,2,3] +[1,2,3] +bool(false) +NULL +NULL