Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#23035 closed defect (bug) (duplicate)

do_action() can't be nested because of global variable

Reported by: cheeserolls's profile cheeserolls Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.5
Component: Plugins Keywords: has-patch
Focuses: Cc:

Description

wp-includes/plugin.php lines 401-408:

reset( $wp_filter[ $tag ] );

do {
	foreach ( (array) current($wp_filter[$tag]) as $the_ )
		if ( !is_null($the_['function']) )
			call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));

} while ( next($wp_filter[$tag]) !== false );

The loop relies on the internal pointer of a global variable: $wp_filter[$tag]

This means that an action cannot be nested inside the same action, because nested calls to do_action will try to use the same pointer for the loop.

For example:
The WPML plugin (wordpress multilingual) maintains duplicates of posts which haven't yet been translated. Whenever you update the main post, WPML will also update the duplicates for you.

It does this using the 'save_post' action. When the 'save_post' action occurs, WPML loops through the duplicated posts and saves them as well. However, saving the duplicates, causes the 'save_post' action to be triggered again.

$wp_filtersave_post? is reset and looped for each of the duplicates. When all the duplicates are done, the loop exits and we return to the outer 'save_post' action. But the inner loop and outer loop share the same pointer. So the outer loop thinks it is finished, and exits. Any other 'save_post' actions that were queued up for the main post don't get executed.

Suggest instead the following patch, to ensure that each call to do_action gets it's own private loop:

$this_filter = $wp_filter[$tag]; // get a copy of the array to loop over
reset( $this_filter );

do {
	foreach ( (array) current($this_filter) as $the_ )
		if ( !is_null($the_['function']) )
			call_user_func_array($the_['function'], array_slice($args, 0, (int) $the_['accepted_args']));

} while ( next($this_filter) !== false );

Change History (3)

#1 follow-up: @toscho
12 years ago

  • Cc info@… added

However, saving the duplicates, causes the 'save_post' action to be triggered again.

But that’s a bug in the plugin code, not in WordPress. To avoid that just remove the action on the first call inside of the function body:

remove_action( current_filter(), __FUNCTION__ );

#2 @SergeyBiryukov
12 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#3 in reply to: ↑ 1 @cheeserolls
12 years ago

Replying to toscho:

However, saving the duplicates, causes the 'save_post' action to be triggered again.

But that’s a bug in the plugin code, not in WordPress. To avoid that just remove the action on the first call inside of the function body:

remove_action( current_filter(), __FUNCTION__ );

Not a bug in the plugin code. The plugin code just calls wp_update_post(). How can a plugin author be expected to know that wp_update_post() will break the currently executing queue of actions?

Anyway, you're talking about a specific example. Even if the specific plugin was patched, the general problem still exists. You should be able to safely call standard wordpress functions within an action, without any danger that the current action will break.

Sergey correctly identified this as a duplicate of http://core.trac.wordpress.org/ticket/17817 where foo123 proposed essentially the same solution as me.

Note: See TracTickets for help on using tickets.