Hi
Thank you for your RFC. This time I'm making sure to comment already
during the discussion period :-)
On 12/5/24 21:57, Eric Norris wrote:
https://wiki.php.net/rfc/error_backtraces_v2
The RFC currently has two voting choices, the first is to accept an
INI setting to configure an error mask for backtraces, and the second
is to choose the default value for the error mask.
You are not restricted to Yes/No votes for the voting widgets. I suggest
to rephrase the second voting widget to:
"What should the default value of the 'error_backtrace_recording' INI be?"
- E_FATAL_ERRORS
- 0
This avoids the complicated sentence above the voting widget.
The “php.ini Defaults” section should probably refer to the voting
widget as well.
The first voting widget would probably be better phrased as:
"Add the 'error_backtrace_recording' INI as described?"
---
As for the RFC contents, I had a look at the previous discussion. Dan
remarked that adding the backtrace to `error_get_last()` would keep any
parameters to functions in the stack trace alive until the next error
occurs. This can easily break RAII patterns, such as this example:
https://3v4l.org/i6EOv
<?php
class Lock
{
public function __construct() { echo "Locked\n"; }
public function __destruct() { echo "Unlocked\n"; }
}
function do_while_locked(Lock $lock)
{
// The exception implicitly keeps the lock alive.
$GLOBALS['x'] = new Exception();
echo "Doing something\n";
}
function foo()
{
$lock = new Lock();
do_while_locked($lock);
echo "Done\n";
}
foo();
echo "After\n";
Normally one would expect the "Unlocked" to be printed between "Done"
and "After", but this is not the case, because the Exception in the
global keeps a reference to $lock within its backtrace. This would be
the same if the Exception would instead be a `trigger_error()` that
remains available via `error_get_last()`.
In other words, we would have a INI setting that changes application
behavior in subtle ways, which is a no-no.
For the previous RFC this apparently was a non-issue, because the stack
was stored as a string. But for your RFC this is an issue, because based
on the tests it's stored as an array.
At the very least this gotcha should clearly be noted in the RFC text.
Best regards
Tim Düsterhus
