View Issue Details

IDProjectCategoryView StatusLast Update
0002222XdebugTracingpublic2024-01-25 08:35
ReporterDriskell Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status newResolutionopen 
Platformx86_64OSCentOSOS Version7
Product Version3.3.0 
Summary0002222: Xdebug holds a reference to objects in the stack trace of a caught exception
Description

I think relating to the 3.3.0 release which fixed some issues with exception.

I am now seeing in a Drupal context that objects are not getting their destructor called sometimes when they should. With Xdebug disabled it calls them normally at the end of the method that created them when their variable goes out of scope.

I think it's related to exceptions being rethrown in some cases perhaps it is "capturing" a reference in a stack and then preventing it from destructing.

Steps To Reproduce
<?php

function innerthrow()
{
    throw new Exception('inner');
}

function rethrower()
{
    try {
        innerthrow();
    } catch (Exception $e) {
    }
}

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

function methodcall()
{
    $destructing = new destructingclass();
    rethrower();
}

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

With Xdebug enabled:
METHOD RETURNED
DESTRUCTION

Without Xdebug:

DESTRUCTION
METHOD RETURNED

Additional Information

This causes some issues in my case because the object not destructing is a transaction object. And due to destructor order not determined on shutdown most of time the connection is destroyed before the transaction is destroyed and so it causes data loss in my test environment with Xdebug enabled.

TagsNo tags attached.
Operating System
PHP Version8.1.10-8.1.19

Activities

Driskell

2023-12-05 14:26

reporter   ~0006703

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

Driskell

2023-12-05 15:07

reporter   ~0006704

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

alexpott

2023-12-07 09:12

reporter   ~0006709

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.

derick

2023-12-07 16:47

administrator   ~0006710

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

Driskell

2023-12-07 17:03

reporter   ~0006711

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?

derick

2023-12-07 18:46

administrator   ~0006712

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

Driskell

2023-12-07 21:59

reporter   ~0006713

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

Driskell

2023-12-07 22:17

reporter   ~0006714

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?

dtakken

2023-12-24 11:38

reporter   ~0006741

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!

Driskell

2024-01-25 08:35

reporter   ~0006800

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.

Issue History

Date Modified Username Field Change
2023-12-05 14:25 Driskell New Issue
2023-12-05 14:26 Driskell Note Added: 0006703
2023-12-05 15:07 Driskell Note Added: 0006704
2023-12-07 09:12 alexpott Note Added: 0006709
2023-12-07 16:47 derick Note Added: 0006710
2023-12-07 17:03 Driskell Note Added: 0006711
2023-12-07 18:46 derick Note Added: 0006712
2023-12-07 21:59 Driskell Note Added: 0006713
2023-12-07 22:17 Driskell Note Added: 0006714
2023-12-24 11:38 dtakken Note Added: 0006741
2024-01-25 08:35 Driskell Note Added: 0006800