Non blocking wp-cron.php with LSAPI

[maximej]

As a follow-up to that change [44488] (Make wp-cron.php non-blocking where possible)

Currently, it's only done with FastCGI, but it's also possible with LiteSpeed LSAPI using litespeed_finish_request().

fastcgi_finish_request() was an alias for a short time in LSAPI but it's not anymore, so wp-cron.php is blocking with LSAPI.

Could we consider implementing it in wp-cron.php?

Others already done that: ​https://github.com/symfony/symfony/pull/42332
and I don't see any issues using it in wp-cron.php.

[johnbillion]

Welcome @maximej and thanks for the ticket.

This sounds like it makes sense. PRs are welcome against ​https://github.com/WordPress/wordpress-develop.

Could you provide the testing steps necessary please?

[maximej]

Thanks @johnbillion, will do a PR soon (not familiar with the testing steps however).

But before, it leads me to another point :
With #47396 and the following changesets [45560] and [46458]
no cache headers are sent for these "early finished requests"

if ( ! headers_sent() ) {
  header( 'Expires: Wed, 11 Jan 1984 05:00:00 GMT' );
  header( 'Cache-Control: no-cache, must-revalidate, max-age=0' );
}

But it was stated that calls to wp-cron.php are not intended to be cached, so why sending these headers only on that non blocking case ?
wp-cron.php can be cached if hosted on incompatible servers.

Do I miss something @peterwilsoncc?

I get that

the fastcgi_finish_request() call prevents the setting of further HTTP headers by caching plugins subsequently loaded.

but no non caching headers are sent afterwards by default.

LSAPI support added for a non-blocking wp-cron.php
Headers intended to prevent caching moved outside of the non-blocking scenario, so that they are also sent if non-blocking isn't supported

Trac ticket: https://core.trac.wordpress.org/ticket/54668

[maximej]

Hi @johnbillion @peterwilsoncc
Sorry if I am bothering you, but any news on that being considered?

Support for non blocking in LSAPI set appart, the fact that headers to prevent caching are only sent with FastCGI looks like a problem to me. Do I miss something?

[peterwilsoncc]

@maximej I've just updated your pull request to resolve some merge conflicts.

I've put this on the milestone for the next major version of WordPress.

Do you know which version of the LSAPI removed the fastcgi_finish_request() alias? It's not in the release log and I am wondering if there was a bug with ending requests early that the developers were avoiding.

[peterwilsoncc]

I get that (fastcgi_finish_request prevents setting further headers but) no non caching headers are sent afterwards by default.

Sorry, I missed this question earlier today.

The idea behind placing the headers inside the fastcgi_finish_request was to ensure that any full page caching plugins retained control of how to manage the wp-cron cache. Most of them do a check for DOING_CRON === true and send out their own headers.

To avoid repeating code multiple times, I think the change you have in your pull request is for the best. Caching plugins calling header() with expiry headers later in the process will replace the existing headers.

[peterwilsoncc]

In 53653:

Cron API: Make wp-cron non-blocking for LiteSpeed LSAPI.

This should make cron spawning faster on LSAPI by ensuring that cron requests return immediately.

To avoid code repetition, the no caching headers are relocated and always sent. Caching plugins attempting to set these
headers later will replace those set by WordPress Core.

Follow up to [44488].

Props maximej, johnbillion.
Fixes #54668.

Merged in 1ff92618f22b92683ca85bff30f368f76d86d9c5 -- canonically https://core.trac.wordpress.org/changeset/53653

[maximej]

Thanks for the change and the explanation @peterwilsoncc !

Concerning the alias removal, here is the according commit
​https://github.com/php/php-src/commit/ccf051c317e606c2a4d9099c6e79a5c42bfdb298
just a few days after they released litespeed_finish_request()

Apparently, there was indeed a bug, fixed in that commit. That may be why they removed the alias, as a precaution.
It was stated "for now" in the commit, but the alias actually never went back in.