wp-background-processing icon indicating copy to clipboard operation
wp-background-processing copied to clipboard

Double dispatch loop?

Open tribulant opened this issue 8 years ago • 5 comments

It seems like WP_Background_Process calls method handle() which calls dispatch() where a parent::dispatch() is also done which in it's own turn fires handle() again. I can do more thorough testing on this but basically handle() is called twice in a single process and could cause some overload.

tribulant avatar Dec 28 '16 14:12 tribulant

I've checked, but that doesn't seem to be the case. Can you confirm?

A5hleyRich avatar Dec 30 '16 21:12 A5hleyRich

Thanks for checking.

Say the original WP_Background_Process class is used to push items to the queue and dispatch the queue to schedule the WordPress cron job, the following happens:

  1. The cron hook is wp_background_process_cron and an action is added to it which fires handle_cron_healthcheck(). Then it fires handle() once it has checked if the process is not already running and if the queue is not empty.
  2. When handle() is done and the queue is not empty when it is finished, it fires the dispatch() method in the WP_Background_Process class again which schedules the cron again. It also calls parent::dispatch() which is the method on WP_Async_Request
  3. The dispatch() method on WP_Async_Request does a wp_remote_post() with action argument equal to the identifier which is wp_background_process
  4. WP_Async_Request has an active Ajax action in place on wp_ajax_wp_background_process in it's constructor which fires it's own method maybe_handle() . This will then fire handle() again which is overridden by WP_Background_Process

So it creates some sort of infinite loop and brings the server down.

The solution for me was to override the dispatch() method in WP_Background_Process in my own child class like WP_Queue_Process as in your example and remove the parent::dispatch() part to eliminate the remote post.

I may still be wrong about this but I found the issue while trying to debug the high load and bottleneck on server processes

tribulant avatar Jan 03 '17 09:01 tribulant

When dispatch is called it schedules the cron health check, but the handle_cron_healthcheck action doesn't fire on the same request, therefore it shouldn't double dispatch. Even if a double dispatch were to occur the process locking would prevent any processing to occur. The duplicate dispatched process would just die gracefully.

The solution for me was to override the dispatch() method in WP_Background_Process in my own child class like WP_Queue_Process as in your example and remove the parent::dispatch() part to eliminate the remote post.

If you're doing this you're queues will be slower because the queue will only process on cron tick, which defaults to 5 minutes.

A5hleyRich avatar Jan 05 '17 15:01 A5hleyRich

Thanks got it. If I find anything I'll let you know. Great library well done

tribulant avatar Jan 05 '17 20:01 tribulant

I was having the same issue but turns out the problem was caused because I was calling the dispatch from the init wp hook without any kind of verification (I didn’t used a link with a nonce like the example). I fixed it without having to override the dispatch() method, by checking if the current request was being called by cron or it is the own action I defined, like that.

<?php

add_action( 'init', 'customplugin_init' );

function customplugin_init() {
  if ( isset( $_GET['doing_wp_cron'] ) ) {
    return;
  }
  if ( isset( $_GET['action'] ) && $_GET['action'] == 'customprefix_customaction' ) {
    return;
  }
  $process = new Custom_Process_Class();

  $process->dispatch();
}

danillonunes avatar Mar 24 '19 01:03 danillonunes

Closing ancient issue. :smile:

ianmjones avatar Apr 11 '23 15:04 ianmjones