[Из песочницы] 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: %d\n", 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: %d\n", 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: %d\n", 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
doPrintCount();
}
private function doPrintCount() {
static $count = 0;
printf("%s: %d\n", 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.
Почему же баг в коде нашего проекта раньше никак не давал о себе знать? Видимо, сущности редко создавались разнотипными группами, а если и создавались — то рефакторили их одновременно. А вот при прогоне тестов в один запуск скрипта попали два объекта, работающие через разные механизмы, и один из них испортил другому состояние.
Выводы
(ведь в каждой серьёзной статье должны быть выводы)
- Статические переменные — зло.
Ну то есть как и любое другое зло в программировании, они требуют осторожного и вдумчивого подхода. Конечно, можно критиковать нас за использование скрытого состояния, но при аккуратном применении это позволяет писать достаточно эффективный код. Однако за static’ами могут скрываться подводные камни, один из которых я вам продемонстрировал. Поэтому - Пишите юнит-тесты.
Никто не поручится, что скрытый косяк в вашем коде не вылезет на свет после очередного рефакторинга. Так что пишите тестируемый код и покрывайте его тестами. Если бы подобный описанному мной баг возник в боевом коде, а не в тестах, на его отладку вполне мог бы уйти весь день, а не полтора-два часа. - Не бойтесь влезть в дебри.
Даже такая простая штука, как статические переменные, может послужить поводом для того, чтобы глубоко погрузиться в системную документацию и исходники PHP. И даже что-то в них понять.
На этой воодушевляющей ноте я прощаюсь с вами. Надеюсь, что эта статья поможет кому-то избежать тех граблей, на которые наступила мы. Спасибо за внимание!
P.S.: Благодарю Романа parpalak за ценные советы при подготовке материала.
