Welcome to ShenZhenJia Knowledge Sharing Community for programmer and developer-Open, Learning and Share
menu search
person
Welcome To Ask or Share your Answers For Others

Categories

Summary

I'm working with a legacy codebase. Refactoring a particular class leads to lockup (website never loading, tests getting stuck). Attempts to track a problematic piece of code with Xdebug trigger heisenbug behavior. Dissecting the changes leads to a property assignment in constructor that triggers the hang.

Many details below are probably irrelevant, but due to the weird nature of this problem I don't know what's actually relevant and what's not, so I'm trying to not omit anything important. So please bear with me.

Original situation

It's a classic website, with PHP rendering whole pages. The codebase is old, doesn't use any framework and doesn't follow good practices or patterns. We're aware of major problems with this app, but it's in the process of being replaced and we're doing only minimum necessary maintenance until we're done with it.

The particular class I'm trying to refactor is a wannabe singleton wrapper for a DBAL connection. Here's a small part of the original class: (original is 240 lines long)

final class MysqliConnection implements Connection
{
    private static $connection;

    private function __construct(DbalConnection $connection)
    {
        self::$connection = $connection;
    }

    public static function fromDefaults(bool $forceReconnect = false): self
    {
        if (self::$connection && !$forceReconnect) {
            return new self(self::$connection);
        }

        return self::fromDefaultConfig();
    }

    // fromDefaultConfig() omitted for brevity
    
    public function fetch(string $sql, ...$params): array
    {
        $stmt = self::tryCall(self::$connection, static function (DbalConnection $connection) use ($sql, $params) {
            // snip...
        });

        // snip...
    }

    // a bunch of instance methods omitted for brevity
}

In a nutshell, all instance methods operate on a static property $connection and there is no instance state at all. The constructor serves no purpose but to assign the static property. This assignment could be moved to fromDefaults(). It's an all-static class in a singleton disguise.

Problem

For reasons, I want to refactor this class to a classic singleton, where the class is actually instantiated with the connection it's supposed to work on. My plan:

  • Add a private property to store the connection
  • Change all instance methods to work with that property rather than the static one
  • Move the static property assignment to fromDefaults()
  • Assign the new property in the constructor

(I am aware that this code isn't exactly equivalent because fromDefaults(true) doesn't affect existing connections anymore, but I'm okay with that.)

To my surprise, as soon as I change the constructor to look like this:

private function __construct(DbalConnection $connection)
{
    $this->instanceConnection = $connection;
}

... PHPUnit starts getting stuck on a not clearly related test and the website is stuck in infinitely loading state in the browser. The only other change I made is moving self::$connection = $connection; to fromDefaults(). $this->instanceConnection isn't used anywhere yet, it's just assigned in the constructor never to be touched again.

Here's (redacted) PHPUnit's output when called with --testdox --debug for extra details:

(... lots of successful tests up until this point ...)
Test 'AppTestsIntegrationUploadTest::testUploadImage' started
Test 'AppTestsIntegrationUploadTest::testUploadImage' ended

 ? Upload image  0 ms
   ┐
   ├ Test skipped because of an error in hook method
   │
   ? /platform-root/app/foo/vendor/phpunit/phpunit/phpunit:61
   ┴
Test 'AppTestsIntegrationUploadTest::testUploadOtherImage' started
Test 'AppTestsIntegrationUploadTest::testUploadOtherImage' ended

 ? Upload other image  0 ms
   ┐
   ├ Test skipped because of an error in hook method
   │
   ? /platform-root/app/foo/vendor/phpunit/phpunit/phpunit:61
   ┴
Test 'AppTestsIntegrationApiFilterTest::testGetFilterByZoneId' started
(... never progresses past this point ...)

(These errors in PHPUnit are expected and not the cause of the problem - they appear with the working version too.)

Further experiments show that:

  • It doesn't matter if $this->instanceConnection is declared in class or not
  • Using different property names doesn't fix the problem
  • Assigning other values to this property works ($this->instanceConnection = null; etc.)
  • Assigning $connection to a local variable works ($test = $connection;)
  • But assigning property through an intermediate variable blocks again ($test = $connection; $this->instanceConnection = $test;)
  • Disabling strict_types doesn't fix the problem
  • Disabling Xdebug in PHP config doesn't fix the problem

Heisenbug behavior with Xdebug

Before finding this particular line that causes the issue, I've been trying to pinpoint it with Xdebug, but it has produced weird results.

If I place a breakpoint at the very beginning of the index.php, it works fine, but a breakpoint roughly in the middle of that file is never reached - the website apparently blocks above it. However, stepping through the code line by line lets me reach that breakpoint with high success rate (not 100% reliably though, there's some randomness to it). Basically watching the code with a debugger makes the problem go away. Larger blobs of code are still causing lockups, but stepping into them and progressing a few lines at a time makes them work again. Looks like a plain old multithreaded deadlock to me, but I don't know how that would be possible in PHP.

The platform

# php -v
PHP 7.4.11 (cli) (built: Oct  6 2020 10:34:39) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.11, Copyright (c), by Zend Technologies
    with Xdebug v2.9.8, Copyright (c) 2002-2020, by Derick Rethans

Running in Docker - custom image based on Debian Bullseye. The issue is reproduced on my Linux machine and in GitLab pipelines.

host$ uname -r
5.4.0-60-generic

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
thumb_up_alt 0 like thumb_down_alt 0 dislike
272 views
Welcome To Ask or Share your Answers For Others

1 Answer

等待大神答复

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
thumb_up_alt 0 like thumb_down_alt 0 dislike
Welcome to ShenZhenJia Knowledge Sharing Community for programmer and developer-Open, Learning and Share
...