PHP, static variables inside class methods and one bug history

In general, I am a frontend developer. But sometimes you have to work with the server part. We have a small team, and when all the real ones backend programmers are busy, it is faster to implement some method myself. And sometimes we sit down together to work on the tasks, so as not to waste time on commiting to and fro. Recently, during one of these rounds of pair programming, a teammate and I ran into a bug that impressed me so much that I decided to share with you.

Bug

So, when after lunch I went to my colleague Roman parpalak , he had just finished tidying up the unit tests, and launched the whole pack. One of the tests threw an exception and fell. Yeah, we thought, now fix the bug. We started the test alone, outside the package, and it was successful.

Before throwing off the afternoon nap, we launched Codeception a few more times. In the package, the test fell, alone passed, in the package fell ...

We got into the code.

The Call to private method crashed from a method that converts an entity object into an array to send to the client. Recently, the mechanism of this process has changed a little, but not all classes have been refactored, so the method should check whether the method that returns the list of required fields (this is the old method) is redefined in the child class. If not, the list of fields is formed through reflection (this is a new method), and the corresponding getters are called. In our case, one of the getters was declared as private, and, accordingly, is not available from the base class. It all looks something like this:

 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 */ ]); } /*   */ } 

As you can see, the result of the reflector is cached in the static $isClientPropsOriginal variable inside the method.

- And what, reflection such a difficult operation? - I asked.
- Well, yes, - Roman nodded.

The breakpoint on the reflection line did not work at all in this class. Never. The static variable was already set to true , the interpreter toClientModelNew into the toClientModelNew method and fell. I offered to see where the assignment occurs:

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

The variable $isClientPropsOriginal was "PaymentList" . This is another class inherited from AbstractEntity , remarkable for exactly two things: it does not override the getClientProperties method and it was tested by a unit test, which has already successfully completed a little earlier.

- How can this be? - I asked. - Is the static variable inside the method being fumbled when inheriting? Why did we not notice this before?

The novel was puzzled as much as mine. While I went for coffee, he sketched a small unit test with an imitation of our class hierarchy, but it did not fall. We missed something. The static variable behaved incorrectly, not in the way we expected, but not in all cases, and we could not understand why. Googling for the query "php static variable inside class method" did not give anything worthwhile, except that static variables are not good. Well, duh!

Now Roman went for coffee, and I thoughtfully opened the PHP sandbox and wrote the simplest code:

simple example 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 

Somehow this should work. The principle of least surprise, all things. But we have because the static variable is defined inside the toClientModel method, and it is overridden in the child class. And what if we write this:

simple example 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 

"How strange," I thought. But there is some kind of logic here. In the second case, the method containing the static variable is called via parent:: , it turns out, is its instance used from the parent class? And how to get out of this situation? I scratched my head and slightly added my example:

simple example 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 

Here it is! The novel had just returned, and I, pleased with myself, demonstrated my achievements. It took him only a few clicks on the keyboard in PHPStorm to refactor the section with a static variable into a separate method:

 private function hasOriginalClientProps() { static $isClientPropsOriginal = null; if ($isClientPropsOriginal === null) { $reflector = new \ReflectionMethod($this, 'getClientProperties'); $isClientPropsOriginal = $reflector->getDeclaringClass()->getName() === 'AbstractEntity'; } return $isClientPropsOriginal; } 

But it was not there! Our error persisted. Looking closely, I noticed that the hasOriginalClientProps method hasOriginalClientProps declared private , in my example it was public . A quick check showed that both protected and public , and private does not work.

simple example 4

 <?php class A { function printCount() { $this->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 

As a result, we declared the hasOriginalClientProps method as protected and supplied with a lengthy comment.

Analysis

Time did not wait, and we moved on to further tasks, but still this behavior was puzzling. I decided to figure out why PHP behaves that way. The documentation failed to dig up anything but obscure hints. Below I will try to restore the picture of what is happening, based on a thoughtful reading of the PHP Internals Book , the PHP Wiki , studying the source code and information on how objects are implemented in other programming languages.

The function inside the PHP interpreter is described by the op_array structure, which, among other things, contains a hash table with static variables for this function. When inheriting , if there are no static variables, the function is reused in the child class, and if there is, a duplicate is created so that the child class has its own static variables in the method.

So far so good, but if we call the parent method via parent::printCount() , then, naturally, we get into the method of the parent class, which works with its static variables. Therefore, example 2 does not work, and example 1 works. And when we brought a static variable to a separate method, as in Example 3, late binding helps us: the A::printCount method will still call a copy of the A::doPrintCount from class B (which, of course, is identical to the original A::doPrintCount ).

To me personally, such copying seemed rather ponderous. Apparently, PHP developers thought the same and refused to copy for private methods. After all, they are still not visible from the child and parent classes! Vaughn, we even caught a futur at the very beginning of the story because of this. Therefore, a private method exists in a single copy throughout the entire class hierarchy, and static variables in it also exist in a single context. Therefore, did not earn an example 4.

This behavior is repeated on all versions of PHP that I tried in the sandbox , starting with shaggy 5.0.4.

Why the bug in the code of our project did not make it known before? Apparently, entities were rarely created with groups of different types, and if they were created, then they refactored them simultaneously. But when running the tests, two objects running through different mechanisms got into one script launch, and one of them spoiled the state for another.

findings

(after all, every serious article should have conclusions)

1. Static variables are evil.
   Well, that is, like any other evil in programming, they require a careful and thoughtful approach. Of course, you can criticize us for using the hidden state, but with careful use this allows you to write quite effective code. However, the static can hide the pitfalls, one of which I showed you. therefore
2. Write unit tests.
   No one can guarantee that the hidden jamb in your code will not come to light after the next refactoring. So write the code under test and cover it with tests. If a bug similar to the one described by me arose in the combat code, and not in tests, it could easily have gone all day, and not one and a half to two hours.
3. Do not be afraid to get into the wilds.
   Even such a simple thing as static variables can be a reason to dive deep into the system documentation and PHP sources. And even something to understand them.

On this inspirational note, I say goodbye to you. I hope that this article will help someone to avoid the rake, which we stepped on. Thanks for attention!

PS: I thank Roman parpalak for valuable advice in preparing the material.