[JoaoMarcos44]

Fixes #25107.

AI-assisted contribution. The explanation, patch, and regression test were prepared with AI assistance. The reported reproductions, build results, and test counts were checked by execution.

Summary

When an array contains deleted entries, array_unshift() can skip a value that a reference foreach has not visited. The fix translates iterator positions after compaction, then applies the existing offset for prepended values.

Infographic

The railway illustration shows which values the iterator visits. The inserted x is not revisited. This is an AI-generated diagram, not a screenshot of test output.

Minimal reproduction

<?php
$items = ['a', 'b', 'c', 'd'];
unset($items[0]);

$seen = [];
foreach ($items as &$item) {
    $seen[] = $item;
    if ($item === 'b') {
        array_unshift($items, 'x');
    }
}
unset($item);

echo json_encode($seen), "\n";

Result	Output
Before the fix	["b","d"]
Expected and after the fix	["b","c","d"]

The PHP 7 foreach RFC and Zend/tests/foreach_016.phpt specify preserving the reference iterator position across array_unshift(). The missing c is an unvisited work item, not an intentionally removed value.

Root cause

Deleting an entry can leave a hole in the internal table. array_unshift() rebuilds the array with only live entries, removing those holes. The previous code adjusted every iterator by argc, but still used the old physical slot position.

In the reproduction, the iterator points to c after the loop has read b:

Step	Next position	Meaning
Old table [hole, b, c, d]	2	Next value is c
Old calculation, 2 + 1	3	Points to d in [x, b, c, d]
Position after removing the hole	1	Position of c among old live entries
Correct calculation, 1 + 1	2	Points to c in [x, b, c, d]

Small implementation and invariants

The patch uses the existing Zend iterator helpers and stays inside array_unshift(). The extra scan runs only when the array has both holes and active iterators. It introduces no helper function, auxiliary map, public API, or additional persistent state.

The remapping has two deliberate steps:

1. Translate each old physical position into its compacted position. While scanning forward, compacted_pos <= idx, so an updated iterator cannot be mistaken for a later old position.
2. Call the existing zend_hash_iterators_advance() once to add the prepend offset.

Applying the prepend offset during each individual remapping could move an iterator onto an old position that has not been processed yet. That iterator could then be moved a second time. Keeping the two operations separate preserves the position invariant without a separate mapping table.

The scan also handles an iterator at a deleted slot and the position just past the old end. Mapping is still required when no values are prepended, because rebuilding can remove holes even with argc == 0.

The implementation is in array.c. The commit also adds a PHPT and a NEWS entry.

Regression coverage

gh25107.phpt checks these cases for both packed and string-keyed arrays:

Case	Behavior checked
Dense array	Existing iterator behavior remains correct
Deleted entry before the iterator	No live value is skipped
One or multiple prepended values	Correct insertion offset
Zero prepended values	Compaction still preserves the next value
A deleted entry at the next position	Iteration continues at the next live entry
Nested iterators at the same or different positions	Each position is preserved independently
Original end followed by an append	The appended value remains reachable

Validation

The target is PHP-8.4, the lowest actively supported affected branch. Validation used PHP 8.4.28-dev on Linux with --enable-debug --enable-zts, based on dfcc0433fcda7fb732cba8d7ee8c857d23f9a9e7.

make -j1
NO_INTERACTION=1 REPORT_EXIT_STATUS=1 TEST_PHP_EXECUTABLE="$PWD/sapi/cli/php" \
  sapi/cli/php -n run-tests.php -q -n -j1 --no-color --no-progress \
  --set-timeout 120 --show-diff ext/standard/tests/array/gh25107.phpt
NO_INTERACTION=1 REPORT_EXIT_STATUS=1 make test \
  TESTS='-j1 -q --no-color --no-progress --set-timeout 120 --show-diff ext/standard/tests/array Zend/tests/*foreach*.phpt'

The unpatched regression skipped c in both packed and mixed arrays. After the patch, it passed 1 of 1. The wider suite reported 889 tests: 876 passed and 13 skipped, with no failures or warnings. The validator checked the actual test count and confirmed execution of gh25107.phpt and Zend/tests/foreach_016.phpt. The build and corrected validator exited 0.

Scope and limits

The sparse-array path adds a scan when active iterators exist. Performance with many simultaneous iterators was not measured. The reasons for the 13 skips were not retained. The full PHP suite and Windows builds were not run.

Related PR #24475 retains the affected fallback for arrays with holes or active iterators. This PR remains a separate bug fix. Local test results do not imply that remote CI has completed.

[arshidkv12]

I think modifying zend_hash_iterators_advance() is the best approach.

ZEND_API void ZEND_FASTCALL zend_hash_iterators_advance(HashTable *ht, HashPosition step)
{
	HashTableIterator *iter = EG(ht_iterators);
	HashTableIterator *end = iter + EG(ht_iterators_used);

	while (iter != end) {
		if (iter->ht == ht) {
			HashPosition pos = iter->pos;
			HashPosition compacted_pos = 0;

			for (HashPosition idx = 0; idx < pos && idx < ht->nNumUsed; idx++) {
				zval *val = HT_IS_PACKED(ht)
					? &ht->arPacked[idx]
					: &ht->arData[idx].val;

				if (Z_TYPE_P(val) != IS_UNDEF) {
					compacted_pos++;
				}
			}

			if (pos > ht->nNumUsed) {
				compacted_pos += pos - ht->nNumUsed;
			}

			iter->pos = compacted_pos + step;
		}
		iter++;
	}
}

[JoaoMarcos44]

@arshidkv12 Thanks for the suggestion. I have now completed the expanded comparison. The performance advantage is not uniform: the proposed helper has substantial wins in some configurations, and substantial regressions in others.

I compared the current patch, your exact helper as a replacement for the remapping in array.c, and your helper with an HT_IS_WITHOUT_HOLES() fast path. The expanded run covered 217 cases × 3 variants × 9 rounds = 5,859 records and 2,456,244 timed calls.

Selected medians below are microseconds per complete array_unshift() call, not timings of the helper alone. Positive changes mean more time than the current patch.

Case	Current patch	Proposed helper	Change	Helper + no-holes guard
Packed, dense, N=1,000, K=8 distributed	2.201	4.372	+98.6%	2.248
Packed, dense, N=1,000, K=128 distributed	2.411	33.785	+1301.1%	2.599
Packed, 25% holes, N=1,000,000, K=8 distributed	6481.338	9447.780	+45.8%	9704.248
Packed, 75% holes, N=1,000, K=128 at the same position	1.945	31.823	+1535.9%	26.869
Mixed, 25% holes, N=64, K=128 distributed	9.070	3.040	−66.5%	2.737
Packed, 25% holes, N=64, K=8 distributed, 256 unrelated iterators	1.719	0.474	−72.4%	0.492

Here K is the number of iterators on the target array. “Unrelated” iterators refer to other arrays but occupy the same global iterator list. N is the physical slot count before deletions.

The last two rows are important counterexamples to a blanket argument against your approach. They favored the proposed helper in all nine paired rounds. Conversely, the same-position sparse case took about 16.36× as much time with the proposed helper, and the guard cannot avoid those repeated prefix scans.

This fits the iterator-adjustment costs:

• The current sparse path scans the array once, but repeatedly scans the global iterator list through lower_pos()/update(): roughly O(N + D*I), where D is the number of distinct target positions and I the global list length.
• The proposed helper scans that global list once, then scans an array prefix for each target iterator: roughly O(I + sum(prefix lengths)). That helps with a large global list or many distinct positions in a small array, but repeats work when many iterators share a position.
• Keeping the no-holes fast path avoids most of the dense-array regressions. It does not resolve the sparse trade-off.

Repeatability of the initial result: I repeated all 34 original configurations. Among the 12 original K=8 configurations, 11 again had a higher median for the proposed helper. The remaining mixed/sparse N=100,000 case was inconclusive: its paired-ratio interquartile range crossed 1 (0.984–1.055), despite a lower marginal median. Thus the earlier “12/12” statement applies to the first run, not universally.

My conclusion is now more specific: performance does not establish a universally preferable implementation. The current patch does well with larger arrays and repeated iterator positions; the proposed helper can do much better with small sparse arrays and many distinct/global iterators. I would preserve the dense fast path whichever direction is chosen. The desired semantics of the exported helper remain a separate design consideration.

Scope, validation, and methodology

• Sizes: 8, 64, 1,000, 100,000 and 1,000,000 physical slots; K=0, 1, 2, 8, 32 and 128; up to 256 unrelated iterators. Multiple hole densities/layouts, early/middle/late and shared/distributed positions, and 0/1/8 prepended values were sampled selectively.
• All three corrected builds passed 15/15 PHPTs and 217/217 full functional preflights, including resuming target and unrelated iterators. The unpatched control reproduced array_unshift() skips unvisited values in foreach by reference when the array has holes #25107.
• PHP 8.4.28-dev release NTS, GCC 12.2 -O2, no OPcache/JIT, Intel i5-13420H under Linux/WSL2. One pinned logical CPU; 2 GiB container cap; PHP memory limit 256 MiB. Base dfcc0433fcda7fb732cba8d7ee8c857d23f9a9e7, current patch 545956c4f430f510deb9ee79c9823fd2ac66e96b. Source hashes and build options matched the first comparison.
• Every timed call used a fresh array. Array construction, deletions, positioning suspended reference-foreach generators, and teardown were outside the clock. No retained array template introduced copy-on-write.
• Nine rounds with balanced variant order; reversed case order on alternating rounds; three discarded warmups per process. Repetitions were fixed per case and equal across variants, ranging from 3 for million-slot cases to 1,000 for small cases.
• Reported percentages compare medians of the nine per-block mean times; they are not medians of paired ratios. All samples were retained. Quartiles are descriptive, not confidence intervals; some no-iterator controls also showed variability.
• This is one-machine microbenchmark evidence. The selected stress cases are not a frequency-weighted production workload, so counting case wins does not identify an overall winner.

Benchmark preparation and analysis were AI-assisted; the builds, tests, and timings were obtained by execution.

[JoaoMarcos44]

Acho que modificar zend_hash_iterators_advance()é a melhor abordagem.

ZEND_API void ZEND_FASTCALL zend_hash_iterators_advance(HashTable *ht, HashPosition step)
{
	HashTableIterator *iter = EG(ht_iterators);
	HashTableIterator *end = iter + EG(ht_iterators_used);

	while (iter != end) {
		if (iter->ht == ht) {
			HashPosition pos = iter->pos;
			HashPosition compacted_pos = 0;

			for (HashPosition idx = 0; idx < pos && idx < ht->nNumUsed; idx++) {
				zval *val = HT_IS_PACKED(ht)
					? &ht->arPacked[idx]
					: &ht->arData[idx].val;

				if (Z_TYPE_P(val) != IS_UNDEF) {
					compacted_pos++;
				}
			}

			if (pos > ht->nNumUsed) {
				compacted_pos += pos - ht->nNumUsed;
			}

			iter->pos = compacted_pos + step;
		}
		iter++;
	}
}

At least, I think it's more favorable in this specific case. However, there may be some other scenario—one that I haven't considered—in which your idea is good or even better.

[arshidkv12]

Note that this function is not used anywhere else.