Sorry the summary needs to be edited - I forgot to update it - it is rather assumptive

If you comment out the throw it will destruct correctly.

I can also confirm that Xdebug 3.2.2 is working. Downgrading to it and running I get the expected DESTRUCTION before METHOD RETURNED. After upgrading again, back to having destruction last.

So this is definitely caused by 3.3.0

This only occurs when develop is set in xdebug.mode.

Whilst I think it is not great practice for code to rely on the order in which objects are desctructed, I think it is a major bug that changing the xdebug.mode results in such differences.

The script can be even simpler:

<?php
class destructingclass
{
    public function __destruct()
    {
        echo 'DESTRUCTION', PHP_EOL;
    }
}

function methodcall()
{
    $destructing = new destructingclass();
    try {
        throw new Exception("TEST");
    } catch (Exception $e) {
    }
}

methodcall();
echo 'METHOD RETURNED', PHP_EOL;

Rethrowing is not required.

The problem is as follows:

Xdebug isn't exactly doing anything wrong here.

What happens, is that Xdebug now keeps a reference to the last 8 exceptions, including their stack trace (with local variables).
Any of these will therefore have their destructor called when Xdebug releases these exceptions. Which can just be at the end of the request.

This script demos that:

<?php
class destructingclass
{
    public function __destruct()
    {
        echo 'DESTRUCTION', PHP_EOL;
    }
}

function methodcall()
{
    $destructing = new destructingclass();
    try {
        throw new Exception("TEST");
    } catch (Exception $e) {
    }
}

methodcall();

echo "Throwing 10 exceptions: \n";
for ($i = 0; $i < 10; $i++) {
    try {
        echo "Throwning exception $i\n";
        throw new Exception("Dummy");
    } catch (Exception $e) {
    }
}
echo "Done throwing 10 exceptions\n";
echo 'METHOD RETURNED', PHP_EOL;

Result:

$ php -dxdebug.mode=develop /tmp/2222.php 
Throwing 10 exceptions: 
Throwning exception 0
Throwning exception 1
Throwning exception 2
Throwning exception 3
Throwning exception 4
Throwning exception 5
Throwning exception 6
Throwning exception 7
DESTRUCTION
Throwning exception 8
Throwning exception 9
Done throwing 10 exceptions
METHOD RETURNED

Thanks @derick I am getting a better understanding here.

Can you let me know why it is remembering the last 8 exceptions? Is this a feature of some kind in 3.3.0?

Yes, this is indeed a new feature. To be able to do these things:

Features:

• Fixed bug 0001562: Add 'local_vars' option to 'xdebug_get_function_stack' to include variables for each st
• Fixed bug 0002194: Add 'params_as_values' option to 'xdebug_get_function_stack' to return data as values
• Fixed bug 0002195: Add 'from_exception' option to 'xdebug_get_function_stack' to return the stack trace where an exception was thrown

Bug fixes:

• Fixed bug 0000450: "Incomplete" backtraces when an exception gets rethrown
• Fixed bug 0000476: Exception chaining does not work properly

I don’t quite understand the 8 exceptions though. Shouldn’t it only keep the exception if it is needed? I don’t quite understand why it keeps an exception referenced (along with local variables etc) if it was caught and then went out of scope. In your simpler example I can’t see any debugging path where you would expect or want or need to know about an exception that was handled. And 8 seems arbitrary? I might be really missing something here but I’m struggling to see it

Ok I think I get it now. The 476 bugfix. Capturing extra info when thrown so you can grab the last transaction in the exception handler and also get lots of additional information not normally stored with the trace. Thus it needs to store it separately to the exception in case it is needed in the handler, as there’s not a way to store it in the exception itself.

Question remaining is would that be best under a feature flag and optin due to the extra references and side effects on destruction behaviour or is it aimed to remain as a default enabled. Maybe an opt out option is useful though?

I got bitten by this exact same issue. I rely on some objects that automatically do cleanup in case something throws an exception.

True, strictly speaking xdebug is not really doing anything that is incorrect. It does not change the fact that object cleanup in PHP is deterministic. However, it does change the behavior of applications in subtle ways. I think xdebug should not do that. At least not by default, and not in ways that are not obvious to the user.

For me it was rather frustrating to trace down why my unit tests started failing. Also, I don't think it is wrong for applications to rely on deterministic object cleanup. In fact, it is an incredibly useful feature of the language, as explained by Johannes Schlüter here: https://schlueters.de/blog/archives/184-Destructing-PHP.html

I'm not sure about the solution though. Would it be possible to create copies of local variables in stead? Or maybe xdebug could check if a variable is an object that should be destroyed (''refcount = 1'') due to it going out of scope?

For now I changed my PHP config to use ''xdebug.mode=off'' by default and use ''xdebug.mode=debug'' when debugging. In both cases object cleanup works as expected.

Thanks for looking into this!

I'm still hitting this periodically across environments - upstream on Drupal and across contrib I think it's going to be a good few years before this gets sorted in that ecosystem and I do agree with the above that this is an application behaviour change and breaks expected PHP behaviour.

@dtakken Thanks for the hint on the xdebug.mode - I will try running things without the "develop" flag for now and hopefully that works :)

Would be really great to understand if there is any leeway on a feature flag here - even if it defaults to on like the feature is now - it would mean we could benefit from all the goodness of the "develop" mode without impacting existing applications by setting the flag.

This bit me too and took quite some time to find the root cause. It's particularly unfortunate that it affects xdebug in its default configuration and that even calling unset on the last reference manually, rather than just relying on it to go out of scope, doesn't prevent the bug. It'd be great if the issue priority could be increased.