Any feedback ?

Here is a simpler example to illustrate the problem. As you see, the 2nd trigger_error will not be caught because the error reporting isn't restored to its initial state.

set_error_handler('handler1', E_ALL);
trigger_error('notice 1',  E_USER_NOTICE); // OK
trigger_error('notice 2',  E_USER_NOTICE); // not caught !

function handler1($errrno, $errstr)
{
    echo "HANDLER1 : $errstr\n";
    set_error_handler('handler2', E_USER_WARNING);
    restore_error_handler();
}

function handler2($errrno, $errstr)
{
}

Same problem in 5.4.8 on CentOS 6.3.

Same problem in 5.3.4 on Windows

Same here for PHP 5.6.14 on Linux

Currently repeats at 5.4, 5.5 and 5.6

Seems fixed in 7.0, hhvm and nightly.

https://travis-ci.org/garex/php-error-handler-bug

Sorry, my bad -- test script was not robust.

Currently it repeats at PHP from 5.2 to nightly (7.x).

Only under hhvm not repeats.

https://travis-ci.org/garex/php-error-handler-bug/builds/135038478

Generally restore_error_handler() does restore the mask as well. What happens here is that while the error handler is called, the error handler is temporarily removed to prevent recursion. The inner set_error_handler() call then thinks that there is no error handler registered and does not save the mask.

Ideally we would not temporarily remove the error handler and instead track that it is currently running in a different way.

The problem is not only about the "error mask". It affects the error-handler itself. Calling set/restore_error_handler() inside the error-handler "corrupts" the "old-error-handler" stack, when there are already >= 2 registered handlers. For future errors, the wrong error-handler is called.

I run into this problem while debugging this issue in Sentry: https://github.com/getsentry/sentry-php/issues/976

Problem still exists in 7.4.3 (and probably "master").

Simpler test-script which also shows the use-case:

Test script:
------------
set_error_handler(function() {
    echo 'First handler' . PHP_EOL;
});

set_error_handler(function() {
    echo 'Second handler' . PHP_EOL;

    set_error_handler(function() {
        echo 'Internal handler' . PHP_EOL;
    });

    $triggerInternalNotice++; // warnings while handling the error should go into internal handler

    restore_error_handler();
});

$triggerNotice1++;
$triggerNotice2++;


Expected result:
----------------
Second handler
Internal handler
Second handler
Internal handler

Actual result:
--------------
Second handler
Internal handler
First handler

I created a draft-pull-request for this issue:
https://github.com/php/php-src/pull/5206

@nikic: Independently from your proposal to not remove the error-handler while error-handling (which also makes sense), I think we always have to push the "empty" handler to the stack to have a consistent behaviour.

This would fix this issue, but I'm not fully aware of the consequences. What do you think?