WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 12 months ago

Last modified 12 months ago

#19818 closed defect (bug) (fixed)

ALTERNATE_WP_CRON causes some cron tasks to fail

Reported by: norocketsurgeon Owned by: ryan
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.3
Component: Cron API Keywords: has-patch commit
Focuses: Cc:

Description

If ALTERNATE_WP_CRON is defined as true and a task run by it requires certain global variables the task will fail. I have traced this issue to a difference in the execution flow between a normal cron call and an ALTERNATE_WP_CRON call. Normal cron calls actually wait for the entire execution flow of require_once('./wp-load.php'); to complete before processing tasks. ALTERNATE_WP_CRON calls result in the execution of the tasks on the action 'sanitize_comment_cookies' which means some globals that may be required are not defined yet. Below is a general outline of the execution flow of the two different types of cron calls.

Normal Cron:

  • 'sanitize_comment_cookies' triggers wp_cron()
  • wp_cron calls spawn_cron()
  • spawn_cron() makes remote post to wp-cron.php on same site
  • wp-cron.php defines 'DOING_CRON'
  • wp-cron.php calls require_once('./wp-load.php');
  • wp-settings.php fires action 'sanitize_comment_cookies'
  • 'sanitize_comment_cookies' triggers wp_cron()
  • wp_cron() calls spawn_cron()
  • spawn_cron() detects 'DOING_CRON' defined and returns
  • the rest of wp-load.php execution finishes resulting in all globals defined
  • wp-cron.php executes tasks

ALTERNATE_WP_CRON:

  • 'sanitize_comment_cookies' triggers wp_cron()
  • wp_cron calls spawn_cron()
  • spawn_cron() requre_once's wp-cron.php
  • wp-cron.php defines 'DOING_CRON'
  • wp-cron.php executes tasks before all globals are ready

I noticed this problem because ping processing requires the global wp_rewrite to be defined in some cases and since this is defined after the 'sanatize_comment_cookies' action is fired, pings to sites with fancy permalink structures will fail. This failure occurs in the call to url_to_postid() in the pingback() function in wp-inclues/comment.php. url_to_postid() attempts to process the pingback link and in cases where there are no post parameters requires $wp_rewrite to be defined.

How to reproduce:

  • define ALTERNATE_WP_CRON to true
  • configure site to send and receive pings and trackbacks
  • create a post with a link to a blog that uses pretty permalinks (http://example.com/example/ would cause the failure but http://example.com/?p=1 would not)
  • publish post
  • no ping will be sent

My idea for a solution would be to put cron task execution inside a function called on the 'wp_loaded' action to ensure everything is setup before running the tasks. I would be willing to implement this if someone more experienced / knowledgeable about cron thinks that this is appropriate.

Attachments (2)

19818.patch (494 bytes) - added by SergeyBiryukov 2 years ago.
19818.02.patch (522 bytes) - added by r-a-y 12 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 solarissmoke2 years ago

Related/duplicate: #19621

comment:2 nacin2 years ago

  • Milestone changed from Awaiting Review to 3.4
  • Version changed from 3.3.1 to 3.3

This is a solid bug report. Should be easy to fix. Thanks!

comment:3 iandunn2 years ago

  • Cc ian_dunn@… added

SergeyBiryukov2 years ago

comment:5 SergeyBiryukov2 years ago

  • Keywords has-patch added

From ticket:9005:14, seems that the only reason wp_cron() is hooked into sanitize_comment_cookies is to load wp-cron.php as early as possible.

Perhaps it can be moved back to init, in order for $wp_rewrite (and $wp_locale) to be set.

comment:6 nacin2 years ago

  • Keywords commit added
  • Owner set to ryan
  • Status changed from new to reviewing

Moving it back to init seems fine.

comment:7 ryan2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In [20652]:

Fire the wp_cron action from init instead of sanitize_comment_cookies so that cron handlers can rely on a fully initialized env. Props SergeyBiryukov, norocketsurgeon. fixes #19818

comment:8 follow-up: r-a-y12 months ago

  • Cc r-a-y@… added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Hi,

We need to run 'wp_cron' later than the default priority of 10 to allow plugins that run on this hook to properly initialize.

In 02.patch, I've bumped the priority to 99.

Let me know what you think.

Update: Created a new ticket for this - #24160 - as advised by Sergey.

Last edited 12 months ago by r-a-y (previous) (diff)

r-a-y12 months ago

comment:9 in reply to: ↑ 8 SergeyBiryukov12 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to r-a-y:

We need to run 'wp_cron' later than the default priority of 10 to allow plugins that run on this hook to properly initialize.

This ticket was closed on a completed milestone. Please open a new one.

Note: See TracTickets for help on using tickets.