Make WordPress Core

Opened 7 years ago

Last modified 5 years ago

#41358 new defect (bug)

Shutdown hooks can significantly slow down REST API responses

Reported by: mikejolley's profile mikejolley Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: REST API Keywords: has-patch dev-feedback needs-testing
Focuses: rest-api, performance Cc:


If you have a site with some slow, maybe deferred actions hooked into shutdown, these actions will slow the response from the REST API unnecessarily.

To test, simply add the following then do a request to the WP Rest API:

add_action( 'shutdown', 'delay_shutdown' );

function delay_shutdown() { sleep( 3); }

The response will not be sent out until all of these shutdown hooks are completed. In our case, we found that some heavy sync functions ran on shutdown would cause our API calls to WooCommerce API timed out, when really the response could have been sent much earlier so these calls could run afterwards.

This is semi-related to in that the fix could be along those lines.

This isn't a solution we feel should be merged into WooCommerce itself because it would be more consistent to have it in WP itself, but see

So on shutdown, it ends any sessions, uses fastcgi_finish_request if available, and falls back to flushing. The response is sent and received, and shutdown continues on in the background. The response is sent before the sleep(3) in this test case.

This could be applied to just REST API requests, or everywhere, depending on everyones feedback. If REST API only, it may make sense to fire an action after the response is sent to avoid shutdown hook altogether.

Attachments (1)

end-rest-api-connection-41358.diff (1.3 KB) - added by mikejolley 7 years ago.
Patch for issue 41358

Download all attachments as: .zip

Change History (10)

7 years ago

Patch for issue 41358

#1 @mikejolley
7 years ago

  • Keywords has-patch dev-feedback needs-testing added

I've added a REST API specific patch since this has the lowest barrier to entry, resolves the problem where it's most noticeable, and should be easier to test.

New method uses fastcgi_finish_request if possible otherwise defaults to sending headers.

To see how much of an improvement it makes, use the code in the original post. If you make a request to the API, before patch you'll wait at least 3 seconds to see the response. After patch, the request is sent instantly.

#2 @johnbillion
7 years ago

I'm going to ping @tellyworth here because he has experience with fastcgi_finish_request() on dot com. Alex, have you seen any problems relating to this?

#4 @jnylen0
7 years ago

I tested a few things locally:

  • On nginx, fastcgi_finish_request is called; error logging during a long shutdown hook seems to work fine, contrary to some of the above reports (may depend on PHP version?)
  • On apache, the logic to send Connection: close and other headers is called, and works fine.

As noted at, on both nginx and apache, outputting after finish_request does nothing, unless output is flushed (using flush() for example), in which case the script will abort. This would be pretty terrible to debug, because the abort doesn't appear to happen immediately.

Fortunately, as noted at, this is fixed for both webservers by calling ignore_user_abort( true ); during finish_request.

I also think we should change the Apache logic a bit: how do we know that ob_get_length() is the correct content length? Maybe we can have serve_request always send a Content-Length header based on the data it echoes, and finish_request do the rest.

#5 @tellyworth
7 years ago

There's a PHP bug that looks relevant:

It's since been fixed, but could be a problem with certain PHP versions.

#6 @rmccue
7 years ago

I'm not sure that this is really REST API-specific. Generally speaking, shutdown hooks will delay any response (and that might be desired behaviour too).

I'd rather have the REST API match regular WordPress behaviour, so I think any solution should be consistent across all of core.

#7 @mikejolley
7 years ago

@rmccue The difference in my eyes is that when the REST api sends the responses, thats it. Nothing else should be sent. Doing so would be invalid.

So this improves performance without affecting other parts of core. I imagine it will take much longer to get approval for a WP-wide change...

#8 @tellyworth
5 years ago

In 44488:

Cron API: Make wp-cron.php non-blocking where possible.

This should make cron spawning faster by ensuring requests to wp-cron.php return immediately regardless of transport method. It is enabled only on recent PHP versions with fastcgi, due to historical bugs and availability of fastcgi_finish_request(). This needs testing on a range of platforms, to help determine if it's safe to use in other contexts also.

Props vnsavage, johnbillion, jnylen0.
See #18738, #41358

#9 @johnbillion
5 years ago

@tellyworth One side effect of calling fastcgi_finish_request() is that any output produced by a shutdown function (or a function hooked to the shutdown action) will not be visible, as the request has already been finished.

I noticed this with another application that was calling fastcgi_finish_request() and caused Query Monitor to break, as its output is produced on the shutdown action and therefore wasn't returned in the response.

This is not a concern for wp-cron.php but it's definitely worth bearing in mind for other contexts.

Note: See TracTickets for help on using tickets.