![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 5 months ago
Last modified 2 months ago
#64653 reviewing defect (bug)
WP_Hook::resort_active_iterations() skips next priority when callback removes itself during execution
| Reported by: | mrcasual | Owned by: | westonruter |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.1 |
| Component: | Plugins | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: |
Description
When a callback removes itself (or is the only callback at its priority) during
do_action()/apply_filters() execution, the next registered priority is silently skipped. This seems to affect all versions since the introduction of WP_Hook.
To reproduce, run this as an mu-plugin:
<?php function wp_hook_bug_self_removing() { remove_action( 'wp_hook_bug_test', 'wp_hook_bug_self_removing', 50 ); error_log( 'Priority 50 executed (removed itself).' ); } add_action( 'init', function () { add_action( 'wp_hook_bug_test', function () { error_log( 'Priority 10 executed.' ); }, 10 ); add_action( 'wp_hook_bug_test', 'wp_hook_bug_self_removing', 50 ); add_action( 'wp_hook_bug_test', function () { error_log( 'Priority 100 executed.' ); }, 100 ); do_action( 'wp_hook_bug_test' ); } );
Expected: all three priorities execute (10, 50, 100).
Actual: priority 100 is silently skipped. Only 10 and 50 execute.
The bug requires all three:
- A callback removes itself (or its entire priority group) during hook execution.
- It is the only callback at that priority (so the priority is fully removed from the array).
- There is at least one priority lower than the removed one. If the removed priority is the lowest, the
array_unshiftbranch inresort_active_iterations()handles it correctly.
In WP_Hook::resort_active_iterations() (wp-includes/class-wp-hook.php), when a priority is removed during iteration, the method rebuilds the iteration array and repositions the internal pointer:
while ( current( $iteration ) < $current ) { if ( false === next( $iteration ) ) { break; } }
This positions the pointer at the first remaining priority >= $current (the removed priority). Since $current no longer exists, the pointer lands on the next priority (e.g., 100).
Back in apply_filters(), the do...while loop calls next() at the bottom of each iteration, which advances past 100. The loop ends, and priority 100 never executes.
Change History (14)
This ticket was mentioned in PR #10949 on WordPress/wordpress-develop by mrcasual.
5 months ago
#1
- Keywords has-unit-tests added
@mrcasual commented on PR #10949:
4 months ago
#3
@westonruter, what's the process for moving this forward?
@westonruter commented on PR #10949:
4 months ago
#5
@mrcasual Once the 7.0 branch is made and trunk is open for commits, we can move this forward for WordPress 7.1.
This ticket was mentioned in PR #11510 on WordPress/wordpress-develop by @liaison.
3 months ago
#6
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.
@liaison commented on PR #11510:
3 months ago
#7
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";
}`
#8
@
3 months ago
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"; }
@mrcasual commented on PR #11510:
3 months ago
#9
@liaisontw, I am not sure I follow the logic behind resubmitting my PR.
@liaison commented on PR #11510:
3 months ago
#10
@liaisontw, I am not sure I follow the logic behind resubmitting my PR. cc @westonruter.
Hi @mrcasual, I apologize for the confusion!
I saw that the CI checks on your PR were showing as 'canceled', and since I confirmed the fix worked locally, I thought something might be broken with the PR itself. I opened a new one just to test if the CI would pass here, but I encountered the same result.
I now realize this might be due to the repo's CI permissions/policy rather than the code. I didn't mean to ignore your work. I'll close this duplicate PR and we can continue the discussion on yours. Sorry for the trouble!
@liaison commented on PR #11510:
3 months ago
#11
@liaisontw, I am not sure I follow the logic behind resubmitting my PR. cc @westonruter.
Hi @mrcasual, I apologize for the confusion!
I saw that the CI checks on your PR were showing as 'canceled', and since I confirmed the fix worked locally, I thought something might be broken with the PR itself. I opened a new one just to test if the CI would pass here, but I encountered the same result.
I now realize this might be due to the repo's CI permissions/policy rather than the code. I didn't mean to ignore your work. I'll close this duplicate PR and we can continue the discussion on yours. Sorry for the trouble!
@liaison commented on PR #11510:
3 months ago
#12
Hi @mrcasual, I apologize for the confusion!
I saw that the CI checks on your PR were showing as 'canceled', and since I confirmed the fix worked locally, I thought something might be broken with the PR itself. I opened a new one just to test if the CI would pass here, but I encountered the same result.
I now realize this might be due to the repo's CI permissions/policy rather than the code. I didn't mean to ignore your work. I'll close this duplicate PR and we can continue the discussion on yours. Sorry for the trouble!
This ticket was mentioned in PR #11715 on WordPress/wordpress-develop by @obenland.
2 months ago
#14
When a callback running inside apply_filters() is the only one at its priority and removes itself (or another callback empties that bucket), WP_Hook silently skips the callbacks at the next remaining priority — but only when at least one earlier-priority callback has already run.
WP_Hook::resort_active_iterations() already has symmetric handling for the add path (a callback added at the current priority during iteration). The remove path was missing the equivalent fix; the trailing next() in apply_filters() overshoots the first remaining priority greater than $current and skips its callbacks.
This patch adds the symmetric counterpart: when $current's bucket has been emptied during iteration, step the iterator back one so that apply_filters()'s trailing next() lands on the right priority.
A regression test is added to tests/phpunit/tests/hooks/removeFilter.php. It fails on trunk without the patch and passes with it.
Trac ticket: https://core.trac.wordpress.org/ticket/64653
## Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.7
Used for: Investigation of the bug, drafting the patch and the regression test, and writing this PR description. The diagnosis, patch shape, and tests were reviewed and edited by me before submission.
Tac ticket: https://core.trac.wordpress.org/ticket/64653
When a callback removes itself during
do_action()/apply_filters()and is the sole callback at its priority,resort_active_iterations()positions the internal array pointer one step too far. The main loop'snext()call then skips the following priority entirely.## Steps to reproduce
The bug requires: (1) a lower priority exists, (2) the removed callback is the only one at its priority.