PHP, статические переменные внутри методов класса и история одного бага

Вообще я разработчик фронтенда. Но порой приходится работать и с серверной частью. Команда у нас небольшая, и когда все настоящиебэкенд-программисты заняты, бывает быстрее реализовать какой-то метод самому. А иногда мы садимся вместе поработать над задачами, чтобы не терять времени на перегон коммитов туда-сюда. Недавно во время одного из таких раундов парного программирования мы с товарищем по команде столкнулись с багом, который меня так впечатлил, что я решил с вами поделиться.

Баг

Итак, когда после обеда я подошёл к своему коллеге Роману parpalak, он как раз закончил приводить в порядок юнит-тесты, и запустил всю пачку. Один из тестов выкинул исключение и упал. Ага, подумали мы, сейчас исправим баг. Запустили тест в одиночестве, вне пакета, и он прошёл успешно.

Прежде чем сбросить с себя послеобеденную дремоту, мы запустили Codeception ещё несколько раз. В пакете тест падал, в одиночку проходил, в пакете падал…

Мы полезли в код.

Фаталка Call to private method вылетала из метода, преобразующего объект сущности в массив для отправки клиенту. Недавно механизм этого процесса немного изменился, но ещё не все классы отрефакторили, поэтому в методе стоит проверка, переопределён ли метод, возвращающий список необходимых полей (это старый способ), в дочернем классе. Если нет, список полей формируется через рефлексию (это новый способ), и вызываются соответствующие геттеры. В нашем случае один из геттеров был объявлен как private, и, соответственно, недоступен из базового класса. Всё это выглядит примерно так:

abstract class AbstractEntity
{
    /* Много кода */

    public function toClientModel()
    {
        static $isClientPropsOriginal = null;

        if ($isClientPropsOriginal === null) {
            $reflector             = new ReflectionMethod($this, 'getClientProperties');
            $isClientPropsOriginal = $reflector->getDeclaringClass()->getName() === 'AbstractEntity';
        }

        if ($isClientPropsOriginal) {
            // TODO В будущем использовать только новую реализацию
            return $this->toClientModelNew($urlGenerator);
        }

        $result = [];
        foreach ($this->getClientProperties() as $clientKey => $property) {
            $value              = call_user_func([$this, 'get' . ucfirst($property)]);
            $result[$clientKey] = $this->formatValueForClient($value);
        }

        return $result;
    }

    public function toClientModelNew()
    {
        $result = [];

        /* Считать аннотации полей класса, получить маппинг полей сущности, сформировать массив данных */

        return $result;
    }

    public function getClientProperties()
    {
        /* Вернуть массив свойств сущности */
    }

    /* Ещё код */
}

class Advertiser extends AbstractEntity
{
    /* Много кода */

    private $name;

    private function getName()
    {
        return $this->getCalculatedName();
    }

    public function toClientModel()
    {
        $result = parent::toClientModel();

        $result['name']    = $this->getName();
        $result['role_id'] = $this->getRoleId();

        return $result;
    }

    public function getClientProperties()
    {
        return array_merge(parent::getClientProperties(), [
            'role_id' => 'RoleId' /* одно из полей для примера */
            /* А name тут нет, он добавляется выше в toClientModel */
        ]);
    }

    /* Ещё код */
}

Как видите, результат работы рефлектора кешируется в статической переменной $isClientPropsOriginal внутри метода.

— А что, рефлексия такая тяжёлая операция? — спросил я.
— Ну да, — кивнул Роман.

Брейкпоинт на строчке с рефлексией вообще не срабатывал в этом классе. Ни разу. Статической переменной уже было присвоено значение true, интерпретатор лез в метод toClientModelNew и падал. Я предложил посмотреть, где же тогда происходит присвоение:

$isClientPropsOriginal = $reflector->getDeclaringClass()->getName() === 'AbstractEntity' ? get_class($this) : false;

В переменной $isClientPropsOriginal стояло "PaymentList". Это ещё один класс, унаследованный от AbstractEntity, примечательный ровно двумя вещами: он не переопределяет метод getClientProperties и он тестировался юнит-тестом, который уже успешно отработал чуть раньше.

— Как такое может быть? — спросил я. — Статическая переменная внутри метода шарится при наследовании? Почему тогда мы раньше этого не заметили?

Роман был озадачен не меньше моего. Пока я ходил за кофе, он набросал небольшой юнит-тест с имитацией нашей иерархии классов, но он не падал. Мы что-то упускали из виду. Статическая переменная вела себя неправильно, не так, как мы ожидали, но не во всех случаях, и мы не могли понять, почему. Гугление по запросу "php static variable inside class method" не давало ничего путного, кроме того, что статические переменные — это нехорошо. Well, duh!

Теперь Роман пошёл за кофе, а я в задумчивости открыл PHP-песочницу и написал самый простой код:

простой пример 1

class A {
    function printCount() {
        static $count = 0;
        printf("%s: %dn", get_class($this), ++$count);
    }
}

class B extends A {
}

$a = new A();
$b = new B();

$a->printCount(); // A: 1
$a->printCount(); // A: 2
$b->printCount(); // B: 1
$b->printCount(); // B: 2
$b->printCount(); // B: 3

Как-то так это и должно работать. Принцип наименьшего удивления, все дела. Но у нас ведь статическая переменная определена внутри метода toClientModel, а он переопределён в дочернем классе. А что, если мы запишем так:

простой пример 2

class A {
    function printCount() {
        static $count = 0;
        printf("%s: %dn", get_class($this), ++$count);
    }
}

class B extends A {
    function printCount() {
        parent::printCount();
    }
}

$a = new A();
$b = new B();

$a->printCount(); // A: 1
$a->printCount(); // A: 2
$b->printCount(); // B: 3
$b->printCount(); // B: 4
$b->printCount(); // B: 5

"Как странно," подумал я. Но какая-то логика тут есть. Во втором случае метод, содержащий статическую переменную, вызывается через parent::, выходит, используется её экземпляр из родительского класса? А как же выйти из этого положения? Я почесал в затылке и немного дополнил свой пример:

простой пример 3

class A {
    function printCount() {
        $this->doPrintCount();
    }
    function doPrintCount() {
        static $count = 0;
        printf("%s: %dn", get_class($this), ++$count);
    }
}

class B extends A {
    function printCount() {
        parent::printCount();
    }
}

$a = new A();
$b = new B();

$a->printCount(); // A: 1
$a->printCount(); // A: 2
$b->printCount(); // B: 1
$b->printCount(); // B: 2
$b->printCount(); // B: 3

Вот оно! Роман как раз вернулся, и я, довольный собой, продемонстрировал свои наработки. Ему понадобилось всего несколько нажатий на клавиатуру в PHPStorm, чтобы отрефакторить участок со статической переменной в отдельный метод:

private function hasOriginalClientProps()
{
    static $isClientPropsOriginal = null;

    if ($isClientPropsOriginal === null) {
        $reflector             = new ReflectionMethod($this, 'getClientProperties');
        $isClientPropsOriginal = $reflector->getDeclaringClass()->getName() === 'AbstractEntity';
    }

    return $isClientPropsOriginal;
}

Но не тут-то было! Наша ошибка сохранялась. Присмотревшись, я заметил, что метод hasOriginalClientProps объявлен как private, в моём примере был public. Быстрая проверка показала, что работают protected и public, а private не работает.

простой пример 4

<?php
class A {
    function printCount() {
        $this->doPrintCount();
    }
    private function doPrintCount() {
        static $count = 0;        
        printf("%s: %dn", get_class($this), ++$count);
    }
}

class B extends A {
    function printCount() {
        parent::printCount();
    }
}

$a = new A();
$b = new B();

$a->printCount(); // A: 1
$a->printCount(); // A: 2
$b->printCount(); // B: 3
$b->printCount(); // B: 4
$b->printCount(); // B: 5

В итоге мы объявили метод hasOriginalClientProps как protected и снабдили пространным комментарием.

Анализ

Время не ждало, и мы перешли к дальнейшим задачам, но всё же такое поведение озадачивало. Я решил разобраться, почему же PHP ведёт себя именно таким образом. В документации не удалось нарыть ничего, кроме неясных намёков. Ниже я попробую восстановить картину происходящего, основываясь на вдумчивом чтении PHP Internals Book, PHP Wiki, изучении исходников и информации о том, как реализуются объекты в других языках программирования.

Функция внутри интерпретатора PHP описывается структурой op_array, которая, среди прочего, содержит хеш-таблицу со статическими переменными этой функции. При наследовании, если статических переменных нет, функция переиспользуется в дочернем классе, а если есть — создаётся дубликат, чтобы у дочернего класса в методе были свои статические переменные.

Пока всё хорошо, но если мы вызываем родительский метод через parent::printCount(), то, естественно, попадаем в метод родительского класса, который работает со своими статическими переменными. Поэтому пример 2 не работает, а пример 1 — работает. А когда мы вынесли статическую переменную в отдельный метод, как в примере 3, нас выручает позднее связывание: метод A::printCount всё равно вызовет копию метода A::doPrintCount из класса B (которая, конечно, идентична оригиналу A::doPrintCount).

Лично мне такое копирование показалось довольно тяжеловесным. Видимо, разработчики PHP подумали так же и отказались от копирования для приватных методов. Ведь они же всё равно не видны из дочерних и родительских классов! Вон, мы даже фаталку в самом начале рассказа словили из-за этого. Поэтому приватный метод существует в единственном экземпляре по всей иерархии классов, и статические переменные в нём тоже существует в единственном контексте. Поэтому и не заработал пример 4.

Такое поведение повторяется на всех версиях PHP, которые я попробовал в песочнице, начиная с мохнатой 5.0.4.

Почему же баг в коде нашего проекта раньше никак не давал о себе знать? Видимо, сущности редко создавались разнотипными группами, а если и создавались — то рефакторили их одновременно. А вот при прогоне тестов в один запуск скрипта попали два объекта, работающие через разные механизмы, и один из них испортил другому состояние.

Выводы

(ведь в каждой серьёзной статье должны быть выводы)

1. Статические переменные — зло.

Ну то есть как и любое другое зло в программировании, они требуют осторожного и вдумчивого подхода. Конечно, можно критиковать нас за использование скрытого состояния, но при аккуратном применении это позволяет писать достаточно эффективный код. Однако за static'ами могут скрываться подводные камни, один из которых я вам продемонстрировал. Поэтому

1. Пишите юнит-тесты.

Никто не поручится, что скрытый косяк в вашем коде не вылезет на свет после очередного рефакторинга. Так что пишите тестируемый код и покрывайте его тестами. Если бы подобный описанному мной баг возник в боевом коде, а не в тестах, на его отладку вполне мог бы уйти весь день, а не полтора-два часа.

1. Не бойтесь влезть в дебри.

Даже такая простая штука, как статические переменные, может послужить поводом для того, чтобы глубоко погрузиться в системную документацию и исходники PHP. И даже что-то в них понять.

На этой воодушевляющей ноте я прощаюсь с вами. Надеюсь, что эта статья поможет кому-то избежать тех граблей, на которые наступила мы. Спасибо за внимание!

P.S.: Благодарю Романа parpalak за ценные советы при подготовке материала.

Автор: Avenger911

Источник