WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 2 weeks ago

#35968 reviewing defect (bug)

wp_reschedule_event timestamp overridden when set in the future

Reported by: svovaf Owned by: DrewAPicture
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.4.2
Component: Cron API Keywords: good-first-bug has-patch
Focuses: docs, performance Cc:

Description

I'm doing some work with WP cron and I noticed two issues in the wp_reschedule_event() code (wp-includes/cron.php).

The first problematic logic is the following:

<?php
        if ( $timestamp >= $now ) {
                $timestamp = $now + $interval;
        } else {
                $timestamp = $now + ( $interval - ( ( $now - $timestamp ) % $interval ) );
        }

Based on the function documentation, $timestamp is the Timestamp for when to run the event.
Let's say that I want to set the first run of the rescheduled event in one hour from now. This if statement overrides the $timestamp based on the interval, practically ignoring my initial $timestamp. If the logic is correct, then something in the documentation should be changed because it's not clear at all.

Btw. I searched trac and found this:
https://core.trac.wordpress.org/changeset/10969

The second issue I see is the last line in that same function:

<?php
        wp_schedule_event( $timestamp, $recurrence, $hook, $args );

Inspecting the code of wp_schedule_event the cron is scheduled based on the given timestamp. What it means is if the originally scheduled task execution time does not match to the new rescheduled $timestamp, it will not override the cron but will add a new one.

Maybe be there's some "dark magic" behind the scenes that makes it all work, but from reading the code + documentation, things doesn't make sense.

Attachments (1)

35968.patch (597 bytes) - added by Dharm1025 5 months ago.
doc updated for $timestamp

Download all attachments as: .zip

Change History (7)

#1 @jrf
2 years ago

Hi @svovaf,

Thanks for reporting this. I've just had a look at this and while I understand what you're saying, I believe this is not a bug.

If you want to schedule a recurring event, you should use the wp_schedule_event() function and set the $recurrence parameter. The $timestamp you provide will be respected.

wp_reschedule_event() is - more than anything - intended for internal use. It is used during the running of a cron job to automatically reschedule a recurring event. In other words, it's called at approximately the time as set in $timestamp when the event was scheduled and used to schedule the next time this event should run.
With that usage in mind, there's nothing really wrong with the code, though I agree the function description could be enhanced a little to highlight the intended use of the function.

#2 @svovaf
2 years ago

Gotcha. It would be great if you guys could update the documentation of the function.

This ticket was mentioned in Slack in #docs by morganestes. View the logs.


17 months ago

#4 @DrewAPicture
6 months ago

  • Keywords needs-patch good-first-bug added

@svovaf @jrf Would either of you like to create a patch for improving the documentation?

@Dharm1025
5 months ago

doc updated for $timestamp

#5 @Dharm1025
5 months ago

  • Keywords has-patch added; needs-patch removed

#6 @DrewAPicture
2 weeks ago

  • Owner set to DrewAPicture
  • Status changed from new to reviewing

Assigning to mark the good-first-bug as "claimed".

Note: See TracTickets for help on using tickets.