wp_ob_end_flush_all() tries to destroy non-destroyable output buffers

[dd32]

wp_ob_end_flush_all() currently tries to flush & destroy every PHP Output buffer that is enabled on the current installation.
However, not every type of PHP Output buffer can be destroyed, which will cause a PHP Notice when attempts to do so are made.
An example of this is PHP extensions such as New Relic, or PHP output buffers created with the $erase flag set to false in ob_start()

An example is when New Relic is installed on a host, also ​see New Relics FAQ entry on the issue.

( ! ) Notice: ob_end_flush(): failed to send buffer of New Relic auto-RUM (1) in ../trunk/wp-includes/functions.php on line 2641
Call Stack
#	Time	Memory	Function	Location
1	0.8510	4328360	shutdown_action_hook( )	../load.php:0
2	0.8510	4328440	do_action( )	../load.php:556
3	0.8510	4329856	call_user_func_array ( )	../plugin.php:406
4	0.8510	4329888	wp_ob_end_flush_all( )	../plugin.php:406
5	0.8511	4330064	ob_end_flush ( )	../functions.php:2641

Somewhat similar to #18239
I'm not sure of what the ideal solution for this would be for WordPress, but wanted to record the issue being known.

[dd32]

One option is to just silence the PHP Notice to get the message out of developers faces.. (Ignore the extra ob in the diff..)

[chriscct7]

Extra item in the docblock

[kkmuffme]

I created a proper fix for this now, please merge.

The fix checks if the ob can be ended or not - and if it can't it aborts (since any parents cannot be ended either, since we can't skip a level).

What do I need to do so this gets included in the next WP version (6.5)?

[kkmuffme]

I guess my PR also fixes these issues:
#22430
#55833
#18525

As well as tons of issues people had but didn't report in trac (see stackoverflow and the first couple pages of Google)

[swissspidy]

The existing patches in those other tickets have lots of discussion around them and vastly different approaches, including things like padding the output etc.

How can testers verify that this patch here addresses those cases as well?

[kkmuffme]

How can testers verify that this patch here addresses those cases as well?

By testing it for those cases? :) Can you please clarify what you mean?

The padding stuff was for some old IE browsers (that PR is 12 years old) and not relevant anymore today (you can check WP core where these comments were removed)

[swissspidy]

Sure, I meant that testing instructions would be helpful for contributors to verify that this patch works as intended.

[kkmuffme]

Multiple possible scenarios. I guess the easiest is to just turn on zlib.output_compression in the ini.

Without server modification, I guess this should work too (would give a notice in current case, but notice is fixed with the PR):

advanced-cache.php

<?php
function my_ob_cb( $output ) {
    return $output;
}

ob_start( 'my_ob_cb', 0, 0 );

[jorbin]

Updating version based on when wp_ob_end_flush_all was introduced since this is not new to 6.5

[jorbin]

This might require building additional docker images for testing, but I think this needs some automated tests before it lands.

[kkmuffme]

@jorbin I added unit tests that show how/that it works as previously with regular obs (= the unit tests that should have existed previously already)
Additionally I added a unit test to show how it works/fixes the issue with non-removable handlers, however this is skipped due to a bug in phpunit itself (see ​https://github.com/sebastianbergmann/phpunit/pull/5852)
However, this shouldn't be a blocker as the unit tests I added show that it still works as it did previously.