Opened 5 weeks ago
Last modified 4 weeks ago
#65474 new enhancement
Flushing response data in wp-cron needs to be optional
| Reported by: | aldavigdis | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Cron API | Version: | trunk |
| Severity: | major | Keywords: | needs-test-info needs-user-docs has-patch |
| Cc: | Focuses: |
Description
A client's site has been having difficulties running wp-cron jobs. Within 1 second, the wp-cron process get killed. This usually happens in unsuspecting places such as the middle of an SQL request or REST API call, but almost always within the first 1-2 seconds.
However, the exact same functions can be run as a foreground process in PHP without issue.
This has caused issues with Action Scheduler, WooCommerce lookup tables have become out of sync and my own ERP sync solution for WordPress stopped sync.
Running the site locally in PHP-FPM does not reproduce the issue.
I have traced the issue to wp-cron executing litespeed_finish_request() when that function is available. However in this case, the LSPHP web server seems to kill the resulting child process within a second from spawning it.
This also seems to cause some memory leaks and swapping in the LSPHP web server.
The issue has been mended by commenting out the following code block in wp-cron.php:
if ( function_exists( 'fastcgi_finish_request' ) ) {
fastcgi_finish_request();
} elseif ( function_exists( 'litespeed_finish_request' ) ) {
litespeed_finish_request();
}
In order to prevent this and similar issues, I suggest adding a new config constant; WP_CRON_FLUSH, defaulting to false to optionally enable flushing wp-cron response data and continuing in the background.
I also suggest WP_CRON_IGNORE_ABORT, defaulting to true to indicate whether or not to call ignore_user_abort( true ).
It has taken me a while to trace the issue to wp-cron and LSPHP as this had all the symptoms of a memory constrained web server.
I have communicated this issue with the relevant hosting provider as I am uncertain if this is isolated to their hosting packages or if this is more widespread. However, I have concluded that is warrants a patch to the WP Core to make flushing response data and spawning a child process using fastcgi_finish_request() or litespeed_finish_request() optional.
An added benefit to this is this makes it easier and more straightforward to debug wp-cron jobs, as WP_DEBUG_DISPLAY can be set to true along with WP_CRON_FLUSH to false. In addition, tasks that should take more than a couple of seconds but crash can be identified using browser tools or cron without running something like New Relic to identify issues.
Draft patch coming soon. I don't expect this to require unit tests.
Attachments (3)
Change History (20)
#2
@
5 weeks ago
Makes sense to me, I’ve been seen weird action scheduler cron issues and they have been hard to debug. This would make it easier to debug things.
#3
@
4 weeks ago
- Milestone Awaiting Review → Future Release
Could you open a pull request with that patch? This will make it easier to review.
This ticket was mentioned in PR #12210 on WordPress/wordpress-develop by @iamchitti.
4 weeks ago
#4
Fixes https://core.trac.wordpress.org/ticket/65474
On some server configurations (notably LiteSpeed with LSPHP), calling
litespeed_finish_request()orfastcgi_finish_request()can prematurely terminate the cron process mid-execution, causing data inconsistencies with plugins like WooCommerce and Action Scheduler.
This PR introduces two opt-out constants, definable in wp-config.php:
WP_CRON_DISABLE_FLUSH— skips the fastcgi_finish_request() / litespeed_finish_request() callWP_CRON_DISABLE_IGNORE_ABORT— skips the ignore_user_abort(true) call
Both default to false, so no behavior change for existing sites. The naming follows existing WordPress conventions (
DISABLE_WP_CRON,WP_DISABLE_FATAL_ERROR_HANDLER).
Note: The original patch on Trac proposed
WP_CRON_FLUSHdefaulting to false, which would have been a breaking change. This PR takes the opt-out approach instead.
#5
@
4 weeks ago
Thanks for the detailed report and the patch, @aldavigdis — this is clearly a real pain point for LiteSpeed environments.
Since Weston suggested continuing the discussion via a PR, I've taken the liberty of opening one based on your work: https://github.com/WordPress/wordpress-develop/pull/12210
The main difference from your patch is the constant naming — I've gone with WP_CRON_DISABLE_FLUSH and WP_CRON_DISABLE_IGNORE_ABORT to follow the opt-out convention used by DISABLE_WP_CRON and WP_DISABLE_FATAL_ERROR_HANDLER, and to ensure no default behavior changes for existing sites. Happy to defer to your original naming if the core team prefers it — the PR is just meant to move the conversation forward.
#6
@
4 weeks ago
@iamchitti I understand the enthusiasm but I submitted this on the morning of a holiday after having to pull an all-nighter having to fix a related issue for a client and given the glacial speeds I tend to see things move at for WordPress Core, I find it super inappropriate for you to make this pull request within ca. 24 hours of me submitting the original Trac ticket.
In order to keep things streamlined, I would be glad if you closed your GH PR and submitted those changes as change proposals the one I will create later today.
#7
@
4 weeks ago
@westonruter Happy to make my own GH PR later by tonight. I want to give this some extra testing and thoughts as well as I have PHP-FPM and LSPHP environments I'd like to test the patch in before it goes through any CI process or further technical discussion.
@aldavigdis commented on PR #12210:
4 weeks ago
#8
I request that this PR is closed until I make my own within a reasonable time frame, based on the original SVN patch at https://core.trac.wordpress.org/ticket/65474.
#9
@
4 weeks ago
@aldavigdis apologises, I was following Weston's suggestion to move the discussion to a PR, but I should have waited for you to take that step yourself — it's your ticket and your patch. My intention was collaboration, not competition, but I clearly jumped ahead. I'll close this PR now and look forward to contributing to yours instead.
@iamchitti commented on PR #12210:
4 weeks ago
#10
Closing based on https://core.trac.wordpress.org/ticket/65474#comment:9
#11
@
4 weeks ago
From @johnbillion on #64157
One complication is that the call to
fastcgi_finish_request()/litespeed_finish_request()that runs prior to wp-load.php being loaded, and therefore prior to when a new constant or filter value can be checked.
As a result some juggling of the code will be required in order to check the new code. Without this juggling, the implementation would require environment variables rather than constants/filters.
My preference would be for some new filters rather than new constants as filters provide more flexibility for developers of WP sites. However, I'm happy to have a discussion about environment variables in order that the code juggling can be avoided.
This ticket was mentioned in PR #12222 on WordPress/wordpress-develop by @aldavigdis.
4 weeks ago
#12
A client's site has been having difficulties running wp-cron jobs. Within 1 second, the wp-cron process get killed. This usually happens in unsuspecting places such as the middle of an SQL request or REST API call, but almost always within the first 1-2 seconds.
However, the exact same functions can be run as a foreground process in PHP without issue.
This has caused issues with Action Scheduler, WooCommerce lookup tables have become out of sync and my own ERP sync solution for WordPress stopped sync.
Running the site locally in PHP-FPM does not reproduce the issue.
I have traced the issue to wp-cron calling litespeed_finish_request() when that function is available. However in this case, the LSPHP web server seems to kill the resulting child process within a second from spawning it.
This also seems to cause some memory leaks and swapping in the LSPHP web server.
The issue has been mended on the client's site by commenting out the following code block in wp-cron.php:
if ( function_exists( 'fastcgi_finish_request' ) ) { fastcgi_finish_request(); } elseif ( function_exists( 'litespeed_finish_request' ) ) { litespeed_finish_request(); }
In order to prevent this and similar issues, I suggest adding a new config constant; WP_CRON_FLUSH, defaulting to true to optionally enable flushing wp-cron response data and continuing in the background.
I also suggest WP_CRON_IGNORE_ABORT, defaulting to true to indicate whether or not to call ignore_user_abort( true ) as it may cause similar issues.
It has taken me a while to trace the issue to wp-cron and LSPHP as this had all the symptoms of a memory constrained web server.
I have communicated this issue with the relevant hosting provider as I am uncertain if this is isolated to their hosting packages or if this is more widespread. However, I have concluded that is warrants a patch to the WP Core to make flushing response data and spawning a child process using fastcgi_finish_reques() or litespeed_finish_request() optional.
An added benefit to this is this makes it easier and more straightforward to debug wp-cron jobs, as WP_DEBUG_DISPLAY can be set to true along with WP_CRON_FLUSH to false. In addition, tasks that should take more than a couple of seconds but crash can be identified using browser tools or cron without running something like New Relic to identify issues.
I don't expect this to require unit tests as this concerns specific application servers.
Trac ticket: https://core.trac.wordpress.org/ticket/65474
## Use of AI Tools
No AI tools used.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
#13
@
4 weeks ago
@peterwilsoncc I suggest not breaking off from current conventions as this is a minor change. As this is both a development and ops issue, I can tell you that sysadmins in the WordPress space are used to juggling WP config constants and using tools such as WP-CLI to manage them.
If we want to use filters, that's the same story. Server-ish configuration belongs in wp-config.php whilst development stuff can use filters. WordPress and some PHP application servers also don't play well with environment variables.
If we cant to change the conventions, then that is a separate ticket.
#14
@
4 weeks ago
(No need to attach patch/diff files to the ticket in addition to opening the PR. We can use the PR now going forward.)
#16
@
4 weeks ago
@aldavigdis The current convention is to use filters whenever possible and avoid the introduction of constants.
However, my main point is that using either prior to the inclusion of wp-load.php is not possible as the call to fastcgi_finish_request()/litespeed_finish_request() occurs before they are available.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Draft patch