dynamically adding/removing filters breaks plugins

[Denis-de-Bernardy]

noticed this while doing something similar to this:

add_action('foo', 'bar1', 10);
add_action('foo', 'bar2', 20);

function bar1() {
  remove_filter('foo', 'bar1', 10);
}

in the above, bar2() doesn't get called. it is because of the usage of next()/current() to loop through filters.

attached patch uses a foreach loop instead, to make it work.

[Denis-de-Bernardy]

the two reset calls probably aren't needed either

[Denis-de-Bernardy]

oh, and, in case you're wondering why this is useful, it's to cope with output buffers, with an ob_start on the one hand side that removes itself (so as to not get triggered more than once), and an ob_flush on the other that removes itself for the same reason.

[dd32]

We've been down this route a few times over the years i think.

See #5338 for the latest such example.

Any changes to the Plugin API would require extensive testing as well.

[Denis-de-Bernardy]

closing as wontfix then. the issue raised against the foreach in that other ticket is the same as mine, but the other way around. arbitration probably goes towards backwards compat.

[hakre]

Related: #10535

[scribu]

Related: #16245

[scribu]

Related: #17817

[solarissmoke]

Akismet is probably the most widely used plugin that does this (calls remove_action on a currently active filter) and it breaks a lot of other plugins that use that filter ( ​Bug report and ​trac ticket).

If this is difficult to fix outright, perhaps we could raise a debug error when remove_action is called on an active filter? At least that way plugin developers would get a warning when they're doing it wrong.

[Chouby]

Just ran into the same issue of a plugin broken by another one due to this.

[SergeyBiryukov]

Related/duplicate: #21169

[nikolov.tmw]

Might be a silly question, but why not just remove

if ( empty( $GLOBALS['wp_filter'][ $tag ][ $priority ] ) ) {
	unset( $GLOBALS['wp_filter'][ $tag ][ $priority ] );
}

From remove_filter()? Because I believe the problem occurs when you remove a priority key(once there are more callbacks for that priority for the given action/filter) from the array.

In my testing commenting-out unset(... fixed the issue altogether.

However I'm not familiar enough with the inner-workings of the filter API to foresee whether doing so will create any side effects.

I believe that having an empty priority key should not be too much of an issue, since we're using foreach( (array) current( $wp_filter[$tag[ )... ) in order to go through the different priorities, which means that if a key is empty, then simply nothing will happen(you'll move to the next priority in the stack).

I was just debugging a conflict between plugins and calling remove_action() was the culprit. To be honest, having a buggy implementation of remove_filter() and remove_action() is not really good(because sometimes you do need to have filters/actions that only run once).

Feedback from someone with more insight would be appreciated :)

[SergeyBiryukov]

#33144 was marked as a duplicate.

[swissspidy]

Is this still a thing with #17817 now fixed?

[Rarst]

Can no longer reproduce as described, think it did get resolved.

[nprasath002]

I was able to reproduce this with the following code

add_action( 'init', 'first_callback', 777 );
add_action( 'init', 'second_callback', 778 );
add_action( 'init', 'third_callback', 779 );

function first_callback() {
	remove_action( 'init', 'first_callback', 777 );
}

function second_callback() {
	die( 'This want get executed' );
}

function third_callback() {
	// This will get executed
}

[adamsilverstein]

I can confirm that testing with the code provided by @nprasath002, the second callback is never executed.