NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[WIP] [Debug] Avoid fatal handler loops#26559

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

Jean85 wants to merge1 commit intosymfony:2.7fromJean85:avoid-fatal-handler-loops

Closed

[WIP] [Debug] Avoid fatal handler loops#26559

Jean85 wants to merge1 commit intosymfony:2.7fromJean85:avoid-fatal-handler-loops

Conversation

Copy link

Contributor

Jean85 commentedMar 16, 2018

Q	A
Branch?	2.7
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	no
Fixed tickets	#26438
License	MIT

This PR starts to try and solve#26438. For now it's just a WIP with the reproducing test case.

The bug is present when:

a third party error handler (like Sentry) is installed
a fatal in thrown under PHP 7

The third party error/exception handlers remember the previous handler, and call them when their work is finished. But Symfony's handler has this piece of code:

symfony/src/Symfony/Component/Debug/ErrorHandler.php

Lines 616 to 635 in874d4d6

	while (!is_array($handler) \|\| !$handler[0]instanceof self) {
	$handler =set_exception_handler('var_dump');
	restore_exception_handler();

	if (!$handler) {
	break;
	}
	restore_exception_handler();

	if ($handler !==$previousHandler) {
	array_unshift($handlers,$handler);
	$previousHandler =$handler;
	}elseif (0 === --$sameHandlerLimit) {
	$handler =null;
	break;
	}
	}
	foreach ($handlersas$h) {
	set_exception_handler($h);
	}

Citing@alcohol from#26438:

it looks like it is trying to find itself (or an instance of) in the stack of exception handlers that may or may not have been registered. It does this by restoring previous error handlers, and looking at what gets restored. Once it finds itself (or there simply arent any), it "restores" the previous stack of error handlers by reassigning them in order, and proceeds to the setExceptionHandler step, where the $handler should be an instance of itself, and $h is whatever what was last added to the stack of error handlers.

This operation creates a loop because the handler is altering the order of exception handlers, generating a loop: Symfony's handler is normally registered first by the FrameworkBundle, so it's putting himself as the last one registered with that.

Pinging@nicolas-grekas since he seems to have wrote a great part of the ErrorHandler.

Add failing test case to reproducesymfony#26438

e978b6a

carsonbot added Status: Needs Review Debug Bug labels

Mar 16, 2018

Copy link

Contributor

alcohol commentedMar 16, 2018

Just to clarify, itlooks like that, but I have no real idea because the code is very non-descriptive. Breaking the code up into smaller methods with clear intent might help resolve the bug itself more easily :-)