Skip to content
This repository has been archived by the owner on Sep 26, 2020. It is now read-only.

Calling fastcgi_finish_request() during a shutdown function prevents subsequent shutdown functions from firing #19

Closed
johnbillion opened this issue Jan 30, 2019 · 3 comments

Comments

@johnbillion
Copy link

In #13, a call to fastcgi_finish_request() was added inside the collector's shutdown function. This unfortunately prevents all subsequent shutdown functions from firing.

I don't fully know why this is the case, but I do know that any shutdown function that exits will cause subsequent ones to not trigger. This may be related.

The bug can be demonstrated by adding the following code anywhere in your PHP application and observing that it does not fire. If the call to fastcgi_finish_request() is removed, it fires as expected.

{{{
register_shutdown_function( function() {
	var_dump( 'Nooooooooooooo' );
} );

cc @benbor

@johnbillion
Copy link
Author

Clarification after reading through https://www.drupal.org/project/drupal/issues/2157053:

The shutdown functions do actually fire, but any output they trigger is not returned to the client due to the fact the request has been finished by the call to fastcgi_finish_request(). This makes sense now I've thought about it.

Clearly a shutdown function shouldn't normally output anything to the client, but there are other debugging tools that do this, for example the Query Monitor plugin for WordPress which can be used in combination with XHGui for performance debugging.

@benbor
Copy link
Contributor

benbor commented Feb 8, 2019

Hi @johnbillion
Yeah... this is interesting. As a workaround, I can propose to add new config value fastcgi_finish_request and call fastcgi_finish_request() function only, if the config exists.
Pay attention to backward compatibility and set fastcgi_finish_request=true by default :)
After that changes, you will able to set fastcgi_finish_request=false for your instance of xhgui-collector
Thanks

@johnbillion
Copy link
Author

That sounds like a good idea @benbor 👍. I'll take a look at updating the PR soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants