I was working on resource streams and more specifically on fopen().
This function throws a warning when failing, in addition to returning false instead of a resource. The unwanted warnings beings a problem I decided to suppress them.
Two possibilities came to my mind: using the error-suppression operator @ or using set_error_handler(). And having been told that @ had not so good performances and posed often more problem than it resolved, I ran a quick benchmark to see how set_error_handler() fared against it.
And here comes the problematic code:
<?php
error_reporting(E_ALL);
function errorHandler(int $errorNumber, string $errorMessage)
{
throw new Exception();
}
$previousHandler = set_error_handler("errorHandler");
$operations = 10000;
for($i = 0; $i < $operations; $i++) {
try{
$inexistant[0];
} catch (Exception $e) {}
}
set_error_handler($previousHandler);
echo 'ok';
Running this simple code will crash the apache server with this message:
[mpm_winnt:notice] [pid 6000:tid 244] AH00428: Parent: child process 3904 exited with status 3221225725 -- Restarting.
After searching, this message means that the server had an access violation error, mainly in the case of reaching the stack size limit. This however should not be the case as this code should not increase the stack size (and in fact, it doesn’t increase the PHP stack frames).
I also tested if the timing was important but even with a sleep of 3ms between each iteration the crash happen after more or less the same number of iteration. This number is around 700 but fluctuate very slightly, sometimes running fine at 704 and sometimes not.
Also, searching on the php bug tracker doesn’t show anything relevant, except maybe for this bug entry which talk about the fact that there is handling around the call to the handling function. This could mean there is a possibility the exception could bypass some handling on the exit of the function, but as I don’t know a thing about the PHP source code, this is pure speculation.
As I would like to propagate the error message properly, the way using set_error_handler() would be the most legible way, but I know I can use error_get_last() and the @ operator to achieve the same goal with a lot more code (as there is multiple functions like fopen() called one after another in the real projet).
So here are the questions: Is this a bug of PHP? Is there a way to circumvent this problem while keeping clear code?
Thanks.
PS: I know the reason for the benchmark is… dubious at best and I should take whatever code is the most legible as long as the performances are viable, but it still made me discover this interesting point of code.
Edit: I forgot to put the versions I tested this against:
- Windows 7, Apache 2.4.38, PHP 7.3.2 via XAMPP
- Windows 7, Apache 2.4.29, PHP 7.2.2 via XAMPP
- Ubuntu Server 18.04, Apache/2.4.29 (Ubuntu), PHP 7.2.15-0ubuntu0.18.04.1
2
Answers
As this was has been confirmed as a PHP bug and the exact reason was found, I'll post both the answers and how to solve this until the bug is solved.
First, here is the bug report: https://bugs.php.net/bug.php?id=77693.
It is a stack overflow caused the capture of the exception context, which in this case include the function calling the error handler as this is in his behavior to capture the parent context. This parent context include the previoulsy caught exception, which is added to the context of the new exception to be caught, and repeating ad vitam aeternam until a crash.
As the reason is clearly determined, the solution is simple: Just add an unset() to the end of the catch block, like so:
Then there is no more problem.
To add to the second question asking for clean alternatives in the case of fopen, and other methods like it, here is a solution:
Both answers have around the same performances, the @ method being 10% slower, when all the error parameters are used in both.
Why go through all this hassle? It would be a lot better to just handle the return value of
false
, since the docs state:Returns a file pointer resource on success, or FALSE on error.
Therefore it should be sufficient to just check if the return value offopen
is false and then continue operation based on that (or throw your own error if necessary).