Opened 8 years ago
Closed 5 years ago
#35968 closed defect (bug) (fixed)
wp_reschedule_event timestamp overridden when set in the future
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.1 | Priority: | normal |
Severity: | normal | Version: | 4.4.2 |
Component: | Cron API | Keywords: | good-first-bug has-patch |
Focuses: | docs | 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 (3)
Change History (15)
#2
@
8 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.
7 years ago
#4
@
6 years ago
- Keywords needs-patch good-first-bug added
@svovaf @jrf Would either of you like to create a patch for improving the documentation?
#6
@
6 years ago
- Owner set to DrewAPicture
- Status changed from new to reviewing
Assigning to mark the good-first-bug
as "claimed".
#7
@
5 years ago
- Focuses performance removed
- Milestone changed from Awaiting Review to 5.0
- Owner changed from DrewAPicture to peterwilsoncc
- Status changed from reviewing to accepted
42897.diff refreshes the existing patch against trunk and expands the function's documentation to include some of @jrf's explanation above.
#8
follow-up:
↓ 9
@
5 years ago
@peterwilsoncc I get the feeling you may have just uploaded the wrong patch to this ticket ?
#9
in reply to:
↑ 8
@
5 years ago
Replying to jrf:
@peterwilsoncc I get the feeling you may have just uploaded the wrong patch to this ticket ?
So do i...
35968.diff refreshes the existing patch against trunk and expands the function's documentation to include some of your explanation above. ;P
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.