Add pre-fire hook for cron

[kkmuffme]

Followup to https://core.trac.wordpress.org/ticket/56048 https://core.trac.wordpress.org/changeset/54258

This adds an additional hook before every cron event to allow for custom code to run before a cron hook

Use cases:

• custom error checking (e.g. if a cron even has no callbacks,...)
• load libraries/functions/classes that are only needed for a specific number of cron events and would otherwise slow down the site/cron - this hook allows you to handle multiple at once which is useful for e.g. dynamic hook names or for cases where you need to run it for a number of hooks (instead of adding 100 add_action with same callback)

Trac ticket: https://core.trac.wordpress.org/ticket/60645#ticket

[audrasjb]

Self assigning for review and tests.

[rajinsharwar]

Added a small space change suggestion, also we will be needing docs for this hook I believe.

[kkmuffme]

will be needing docs for this hook I believe.

Can you clarify which docs you mean? There's already a docblock there?

[oglekler]

I wonder if this hook makes sense. The hooked function itself can load any libraries, objects, classes and whatever, the code needs to be organized this way reducing unnecessary load and moving this action from one hook to another is not making the code itself better. If you have two actions registered in the same big class, it will not make any difference.

[kkmuffme]

That will create tons of redundant code with dynamic hook names, since you'd have to call the same function to load it in each of them.

Additionally, we use this hook for identification of cron hooks that do not have any callbacks registered. This happens a lot with plugins that rename their hooks in updates but forgot to unschedule the old hooks as well as with plugins that are deactivated/uninstalled but don't unschedule their hooks.
There's no other way to do this atm.

This matters, since every hook (even if unused) will write (update) the cron option, which is rather expensive to update as it requires to be updated atomically and have a grace period to not get major race conditions in a multi-server DB multi-tiered object cache set up.

[peterwilsoncc]

I don't really see the need for this filter.

Detecting cron jobs without any callbacks registered can be done via the pre_reschedule_event filter. If no callbacks are registerd then the hook can be removed and rescheduling overridden.

I think the hooked function for the cron job loading the files is the correct approach as each callback should be self-sufficient in the event it is fired directly. To avoid duplicate code a plugin an create a helper function for the purpose.

There are also some backward/forward compatibility issues with WP-CLI to consider. WP-CLI fires the action directly (view ​source code) so there is a risk the new hook would be missed on systems running real cron using the CLI.

[kkmuffme]

Detecting cron jobs without any callbacks registered can be done via the pre_reschedule_event filter. If no callbacks are registerd then the hook can be removed and rescheduling overridden.

Thanks for the suggestion, but it can't be used because:

• the function can be called from arbitrary locations
• there are tons of hooks that will run after that (all of which cannot be used for this, since those hooks are also called in various other cases, not only in processing) that could add an add_action

I think the hooked function for the cron job loading the files is the correct approach as each callback should be self-sufficient in the event it is fired directly.

I think that's a misunderstanding from my original text (it sounds differently in my last comment, which is probably why): the hook is used to LOAD the callback in the first place. (the add action which will run on the cron event isn't loaded at all but only loaded by this new action)
Without the new hook, there's no callback.

This avoids having to instantiate 1000s of classes that may potentially have an add action in cron on every cron request - instead the class will only be instantied if the event using the class/file is actually executed.
Leading to
a) faster cron execution (since that is an issue already, since cron starts to time lag extremely quickly) b) lower PHP memory consumption and
c) avoids executing old code (which has been an issue in CD for us in some cases, since we have a cron timeout of 15 minutes, and some plugins/classes were updated in the meantime, leading to the execution of old code)

Another thing we started to use this hook is to continuously re-check environment conditions - e.g. wp_die if the site suddenly switched to maintenance mode, the DB version changed,... to avoid corrupting data or producing tons of notices as those are regular issues that happen when using an actual PHP-CLI cron.

There are also some backward/forward compatibility issues with WP-CLI to consider

The existing hooks cron_reschedule_event_error and cron_unschedule_event_error are missing in CLI already too, so that wouldn't be an issue.

---

Are there any downsides to adding this action?

[oglekler]

We have 2 days before Beta 1 and no concrete understanding, so, I am moving this ticket to the next milestone for further discussion.

[chaion07]

Thanks @kkmuffme for reporting this. We reviewed this during a recent bug-scrub session. Here's the summary of the discussion for this Ticket:

1. Referring to Peter's comment, he is still not convinced if this hook is needed but happy to help if there is other contributors willing to take this forward.
2. We are also waiting for @audrasjb to Shepard this Ticket closer to a resolution

Props to @peterwilsoncc

Cheers!

[coquardcyr]

Replying to kkmuffme:

Detecting cron jobs without any callbacks registered can be done via the pre_reschedule_event filter. If no callbacks are registerd then the hook can be removed and rescheduling overridden.

Thanks for the suggestion, but it can't be used because:

• the function can be called from arbitrary locations
• there are tons of hooks that will run after that (all of which cannot be used for this, since those hooks are also called in various other cases, not only in processing) that could add an add_action

I think the hooked function for the cron job loading the files is the correct approach as each callback should be self-sufficient in the event it is fired directly.

I think that's a misunderstanding from my original text (it sounds differently in my last comment, which is probably why): the hook is used to LOAD the callback in the first place. (the add action which will run on the cron event isn't loaded at all but only loaded by this new action)
Without the new hook, there's no callback.

This avoids having to instantiate 1000s of classes that may potentially have an add action in cron on every cron request - instead the class will only be instantied if the event using the class/file is actually executed.
Leading to
a) faster cron execution (since that is an issue already, since cron starts to time lag extremely quickly) b) lower PHP memory consumption and
c) avoids executing old code (which has been an issue in CD for us in some cases, since we have a cron timeout of 15 minutes, and some plugins/classes were updated in the meantime, leading to the execution of old code)

Another thing we started to use this hook is to continuously re-check environment conditions - e.g. wp_die if the site suddenly switched to maintenance mode, the DB version changed,... to avoid corrupting data or producing tons of notices as those are regular issues that happen when using an actual PHP-CLI cron.

There are also some backward/forward compatibility issues with WP-CLI to consider

The existing hooks cron_reschedule_event_error and cron_unschedule_event_error are missing in CLI already too, so that wouldn't be an issue.

---

Are there any downsides to adding this action?

I don’t see the value of this new action.

This was justified by two use cases:
The first one is solved by the filter @peterwilsoncc proposed.
The second use case is not really clear.
From what I understood, it seems to be a way to reduce memory consumption and avoid old code usage.
However, I don't really see why checking for this right before firing the cron hook would change anything about theses points as it is exactly the same environment and we could instead directly use the cron hook to make theses checks.

[johnbillion]

I concur with the above that this action doesn't provide any functionality that isn't already possible via other means. Thanks as always @kkmuffme but I'll close this one off!