New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flush backend output buffer after closing. #46931
Conversation
The PHP 8.2 test failure seems unrelated... The Appveyor test seems to target Windows (?) and this is suspicious:
I'll postpone digging too deeply into that until I get feedback on the general approach, though. If this is something Symfony will concern itself with, then we can address that. |
Hey! I think @abdounikarim has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
It looks like we should consider this as a bug, so this should probably be rebased on 4.4? |
Thanks for the review, @fabpot.
I wasn't sure if this was a purposeful omission or not (I erred on the side of yes, it was) so I can recategorize and retarget as a bug. |
Thank you @bradjones1. |
noyesCurrently,
Response::send()
does some backend/server specific shutdown, with the default case being closing userland output buffers. PHP's output buffering may or may not beOn
by default.I recently discovered in the course of debugging my Drupal 9 project that closing output buffers may not be enough to flush the response's content to the client. In my case, output buffering wasn't enabled at all, making
Response::closeOutputBuffers()
a no-op. In either case, it appears necessary to also callflush()
. This is important if the calling script/framework wishes to perform additional work after sending the request but before shutting down.According to the PHP docs for flush(), this seems to be necessary in addition to closing all active output buffers (if used.)
If it is Symfony's intent that the "final"
flush()
call be done by the implementing code/extending framework, it would be good to explicitly note that.