Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite recursion with test doubles in combination with a destructor #5809

Closed
nielsdos opened this issue Apr 10, 2024 · 6 comments
Closed
Assignees
Labels
feature/test-doubles Test Stubs and Mock Objects type/bug Something is broken version/10 Something affects PHPUnit 10 version/11 Something affects PHPUnit 11

Comments

@nielsdos
Copy link

Q A
PHPUnit version 10.5.17
PHP version 8.2.19-dev
Installation Method Composer

Summary

This is related to a php-src bug report: php/php-src#12373
After debugging, I found that this is not a PHP bug, but a PHPUnit bug. The segfault happens because of infinite recursion.

The reproducer (linked below) has a method methodWithSelfReferenceReturn with a return type of self. This method is called from the destructor.
The class is also mocked in the test case, the mock codegen for that method contains a snippet like this:

// Let's call this code "snippet start"
$__phpunit_result = $this->__phpunit_getInvocationHandler()->invoke(
    new \\PHPUnit\\Framework\\MockObject\\Invocation(
        'Demo\\ExampleOne', 'methodWithSelfReferenceReturn', $__phpunit_arguments, 'Demo\\ExampleOne', $this, false
    )
);

Stepping through this the following happens:

  • "snippet start" calls InvocationHandler.php::invoke which calls $invocation->generateReturnValue()
  • In Invocation.php: generateReturnValue() calls (new ReturnValueGenerator)->generate(...)
  • In ReturnValueGenerator.php: call $this->testDoubleFor($returnType, $className, $methodName); because the return type is neither an intersection nor a union.
  • In Generator.php: $object = $this->getObject(...) creates an object of type TestStub_ExampleOne_019e0114.
  • Eventually the object will be destructed and so its destructor will be called, however, the destructor will execute "snippet start" again because it calls methodWithSelfReferenceReturn. This creates an infinite recursion, eventually crashing the PHP interpreter.

If you print out a backtrace using debug_backtrace() from PHP, you'll see over time the number of these entries grow:
"ExampleOne.php","line":14, hinting about the infinite recursion.

Current behavior

Crash due to infinite recursion.

How to reproduce

  1. Clone https://github.com/DragosMocrii/php-bug-destruct-segfault
  2. composer install
  3. php ./vendor/bin/phpunit

Expected behavior

No crash.

@nielsdos nielsdos added the type/bug Something is broken label Apr 10, 2024
@sebastianbergmann sebastianbergmann added feature/test-doubles Test Stubs and Mock Objects version/10 Something affects PHPUnit 10 version/11 Something affects PHPUnit 11 labels Apr 11, 2024
@sebastianbergmann sebastianbergmann self-assigned this Apr 11, 2024
@sebastianbergmann sebastianbergmann changed the title Mocking can create infinite recursion in combination with a destructor Test Doubles can create infinite recursion in combination with a destructor Apr 11, 2024
@sebastianbergmann sebastianbergmann changed the title Test Doubles can create infinite recursion in combination with a destructor Infinite recursion with test doubles in combination with a destructor Apr 11, 2024
@sebastianbergmann
Copy link
Owner

Thank you, @nielsdos, for investigating this.

I have stripped down the reproducer:

composer.json

{
  "require-dev": {
    "phpunit/phpunit": "^10.5"
  },
  "autoload": {
    "classmap": [
      "src/"
    ]
  }
}

src/Example.php

<?php declare(strict_types=1);
class Example {
    public function methodWithSelfReferenceReturn(): self
    {
        return $this;
    }

    public function __destruct()
    {
        $this->methodWithSelfReferenceReturn();
    }
}

tests/ExampleTest.php

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

final class ExampleTest extends TestCase
{
    public function testOne(): void
    {
        $this->createStub(Example::class);
    }
}
$ php-82 ./vendor/bin/phpunit --no-configuration --do-not-cache-result tests 
PHPUnit 10.5.17 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.19-dev

[1]    327489 segmentation fault (core dumped)

@sebastianbergmann
Copy link
Owner

Here is what happens in #5809 (comment):

  • createStub() creates a test stub for Example (the destructor of Example is not stubbed)
  • The destructor of Example is invoked
  • The destructor of Example calls methodWithSelfReferenceReturn()
  • The return value generator is called to generate a return value as no return value was configured for methodWithSelfReferenceReturn() on the test stub
  • The return value generator looks at the return type declaration for methodWithSelfReferenceReturn(), sees self, resolves self to Example, and creates a new test stub for Example
  • And now the infinite recursion starts ...

There is no bug here, though.

The return type declaration self "only" means that an object of the same type is to be returned. It does not mean "self reference".

If you configure methodWithSelfReferenceReturn() on the test double to return a self reference (see below) then the return value generator does not get involved by the invocation handler and there will be no infinite recursion.

tests/ExampleTest.php

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

final class ExampleTest extends TestCase
{
    public function testOne(): void
    {
        $stub = $this->createStub(Example::class);

        $stub
            ->method('methodWithSelfReferenceReturn')
            ->willReturnSelf();
    }
}

@nielsdos
Copy link
Author

Thanks for the great explanation!

@matthewjumpsoffbuildings
Copy link

matthewjumpsoffbuildings commented Apr 11, 2024

I am seeing this issue when running unit tests in Laravel, with my test classes extending Illuminate\Foundation\Testing\TestCase

Is it likely that the base Laravel class incorrectly handles self-references and destructors similar to #5809 (comment), and that is the cause of the segfaults I am seeing? Laravel has a ton of methods that return $this, and none of my own tests I am adding on top of the base Laravel test classes create stubs or test self-return, so it seems like the most likely location of the issue is within the base Laravel class no?

@sebastianbergmann
Copy link
Owner

@matthewjumpsoffbuildings Please raise this question with the vendor of your framework.

@eleftrik
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-doubles Test Stubs and Mock Objects type/bug Something is broken version/10 Something affects PHPUnit 10 version/11 Something affects PHPUnit 11
Projects
None yet
Development

No branches or pull requests

4 participants