Skip to content

Hooks: Fix pointer over-advance in resort_active_iterations()#11510

Open
liaisontw wants to merge 1 commit intoWordPress:trunkfrom
liaisontw:fix/64653-resort-active-iterations
Open

Hooks: Fix pointer over-advance in resort_active_iterations()#11510
liaisontw wants to merge 1 commit intoWordPress:trunkfrom
liaisontw:fix/64653-resort-active-iterations

Conversation

@liaisontw
Copy link
Copy Markdown

Retest PR from @mrcasual:

When a callback removes itself during execution and is the only entry at that priority, the internal array pointer is re-positioned by resort_active_iterations().

Before this fix, the pointer ended up at the next priority, but the subsequent next() call in the main apply_filters() loop would then advance it once more, skipping the next priority entirely.

Calling prev() ensures the pointer is balanced so the next() call lands correctly on the intended priority.

Fixes #64653.

Trac ticket: https://core.trac.wordpress.org/ticket/64653

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props liaison, mrcasual.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@liaisontw liaisontw force-pushed the fix/64653-resort-active-iterations branch 3 times, most recently from cdf86a9 to 6c33a7d Compare April 10, 2026 02:07
@liaisontw
Copy link
Copy Markdown
Author

Test code. Put above wp-includes directory.
`<?php
/*
Trac #64653: Optimized minimal reproduction
*/

define( 'ABSPATH', dirname( __FILE__ ) . '/' );
define( 'WPINC', 'wp-includes' );

require_once ABSPATH . WPINC . '/plugin.php';

echo "--- Starting Hook Test ---\n";

$executed_priorities = [];

// Priority 10
add_action( 'test_hook', function () use ( &$executed_priorities ) {
$executed_priorities[] = 10;
echo "Executed: Priority 10\n";
}, 10 );

// Priority 50
function wp_hook_bug_self_removing() {
global $executed_priorities;
$executed_priorities[] = 50;
echo "Executed: Priority 50 (removing itself...)\n";

remove_action( 'test_hook', 'wp_hook_bug_self_removing', 50 );

}
add_action( 'test_hook', 'wp_hook_bug_self_removing', 50 );

// Priority 100
add_action( 'test_hook', function () use ( &$executed_priorities ) {
$executed_priorities[] = 100;
echo "Executed: Priority 100\n";
}, 100 );

do_action( 'test_hook' );

echo "--- Test Completed ---\n";

if ( ! in_array( 100, $executed_priorities ) ) {
echo "❌ BUG REPRODUCED: Priority 100 was skipped!\n";
} else {
echo "✅ TEST PASSED: Priority 100 was executed.\n";
}`

@liaisontw liaisontw force-pushed the fix/64653-resort-active-iterations branch 3 times, most recently from 7ac631b to 3fa890b Compare April 10, 2026 04:11
When a callback removes itself during execution and is the only entry at that priority, the internal array pointer is re-positioned by resort_active_iterations().

Before this fix, the pointer ended up at the next priority, but the subsequent next() call in the main apply_filters() loop would then advance it once more, skipping the next priority entirely.

Calling prev() ensures the pointer is balanced so the next() call lands correctly on the intended priority.

Fixes #64653.
@liaisontw liaisontw force-pushed the fix/64653-resort-active-iterations branch from 3fa890b to 2565a9a Compare April 10, 2026 06:22
@mrcasual
Copy link
Copy Markdown

mrcasual commented Apr 10, 2026

@liaisontw, I am not sure I follow the logic behind resubmitting my PR. cc @westonruter.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an iteration-pointer edge case in WP_Hook::resort_active_iterations() where a self-removing callback (and only callback at its priority) could cause the next priority level to be skipped during apply_filters()/do_action() execution.

Changes:

  • Adjust resort_active_iterations() to step the internal array pointer back when the current priority was removed, preventing an extra next() advance.
  • Add PHPUnit coverage for self-removing callbacks to ensure subsequent priorities still execute.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/wp-includes/class-wp-hook.php Adjusts iteration pointer handling in resort_active_iterations() to avoid skipping priorities after removal.
tests/phpunit/tests/hooks/doAction.php Adds regression tests for self-removing callbacks at different priority positions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +153 to +154
//If the current priority was removed, step back so the next() call in the main loop lands correctly.
if ( false !== current( $iteration ) && current( $iteration ) !== $current ) {
Comment on lines +329 to +332
$hook->do_action( array() );

$this->assertSame( array( 10, 50, 100 ), $log, 'Priority 100 should not be skipped when priority 50 removes itself during iteration.' );
}
Comment on lines +361 to +362


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants