Make WordPress Core

Opened 12 years ago

Last modified 7 days ago

#22239 new defect (bug)

wp_ob_end_flush_all() tries to destroy non-destroyable output buffers

Reported by: dd32's profile dd32 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.2
Component: Bootstrap/Load Keywords: has-patch needs-testing needs-unit-tests
Focuses: Cc:

Description

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.

Attachments (1)

22239.diff (645 bytes) - added by dd32 12 years ago.

Download all attachments as: .zip

Change History (18)

#1 @dd32
12 years ago

  • Summary changed from wp_ob_end_flush_all() tries to flush non-flushable buffers to wp_ob_end_flush_all() tries to destroy non-destroyable output buffers

@dd32
12 years ago

#2 @dd32
12 years ago

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

#3 @nacin
10 years ago

  • Component changed from Warnings/Notices to Bootstrap/Load

#4 @chriscct7
9 years ago

  • Keywords has-patch needs-refresh added

Extra item in the docblock

This ticket was mentioned in PR #5771 on WordPress/wordpress-develop by @kkmuffme.


5 months ago
#5

  • Keywords needs-refresh removed

#6 @kkmuffme
5 months ago

  • Version set to trunk

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)?

#7 @kkmuffme
5 months ago

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)

Last edited 7 weeks ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core by kkmuffme. View the logs.


4 months ago

#9 @swissspidy
4 months ago

  • Milestone set to Future Release

#10 @swissspidy
4 months ago

  • Keywords needs-testing added

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?

#11 @kkmuffme
4 months ago

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)

#12 @swissspidy
4 months ago

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

#13 @kkmuffme
4 months ago

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 );

#14 @jorbin
7 weeks ago

  • Version changed from 6.5 to 2.2

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

This ticket was mentioned in Slack in #core by kkmuffme. View the logs.


7 weeks ago

This ticket was mentioned in Slack in #core-test by ankit-k-gupta. View the logs.


7 days ago

#17 @jorbin
7 days ago

  • Keywords needs-unit-tests added

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

Note: See TracTickets for help on using tickets.