__destruct() and __call() create endless loop

Question

I simplified my code a lot, but what I am doing is something like this:

class App{

    protected $apps = [];

    public function __construct($name, $dependencies){
        $this->name = $name;

        $apps = [];
        foreach($dependencies as $dependName){
            $apps[$name] = $dependName($this); // returns an instance of App
        }
        $this->apps = $apps;
    }

    public function __destruct(){
        foreach($this->apps as $dep){
            $result = $dep->cleanup($this);
        }
    }

    public function __call($name, $arguments){
        if(is_callable([$this, $name])){
            return call_user_func_array([$this, $name], $arguments);
        }
    }
}


function PredefinedApp(){
    $app = new App('PredefinedApp', []);

    $app->cleanup = function($parent){
        // Do some stuff
    };
    return $app;
}

I then create an app like this:

$app = new App('App1', ['PredefinedApp']);

It creates an App instance, and then the items in the array create new app instances of whatever is defined and place them into an internal app array.

When I execute my destructor on the main app, it should call cleanup() on all the child apps. But what is happening, is that it is executing an endless loop, and I am not sure why.

I do notice that if I comment out call_user_func_array then __call() is only called once, but it doesn't execute the actual closure then.

I have also noticed, that if I do a var_dump() in the __call() it dumps endlessly. If I do a var_dump() in cleanup() instead I get a http 502 error.

Answer 1

Replacing your __call() function with the following will prevent the recursion that you are seeing:

public function __call( $method, $arguments ) {
    if ( $this->{$method} instanceof Closure ) {
        return call_user_func_array( $this->{$method}, $arguments );
    } else {
        throw new Exception( "Invalid Function" );
    }
}

---

See @Rizier123's answer for more detail, but the TLDR version is:

The problem is that you initial invoke cleanup() as a method call with $dep->cleanup() which invokes __call(). If you then use call_user_func_array() and pass [$this, $method] it calls it as a method, thus invoking _call() again and triggering the loop (and never actually calling the cleanup() method), whereas using $this->{$method} will invoke it as a Closure and prevent the looping.

Answer 2

So let us go through the code and look what happens here and why:

01|    class App{ 
02|
03|        protected $apps = [];
04|
05|        public function __construct($name, $dependencies){
06|            $this->name = $name;
07|
08|            $apps = [];
09|            foreach($dependencies as $dependName){
10|                $apps[$name] = $dependName($this);
11|            }
12|            $this->apps = $apps;
13|        }
14|
15|        public function __destruct(){
16|            foreach($this->apps as $dep){
17|                $result = $dep->cleanup($this);
18|            }
19|        }
20|
21|        public function __call($name, $arguments){
22|            if(is_callable([$this, $name])){
23|                return call_user_func_array([$this, $name], $arguments);
24|            }
25|        }
26|    }
27| 
28|    function PredefinedApp(){
29|        $app = new App('PredefinedApp', []);
30|
31|        $app->cleanup = function($parent){
32|            //Do stuff like: echo "I'm saved";
33|        };
34|        return $app;
35|    }
36|  
37|    $app = new App('App1', ['PredefinedApp']);

_{Note: Line numbers added for each line of the code, so I can reference to these lines in the answer below}

Problem

1. You create an instance of: App with the following line:

   $app = new App('App1', ['PredefinedApp']);  //Line: 37

2. The constructor gets called:

   public function __construct($name, $dependencies){ /* Code here */ }  //Line: 05

   2.1. Following parameters are passed:

   • $name = "App1"
   • $dependencies = ["PredefinedApp"]

3. You assign $name to $this->name with this line:

   $this->name = $name;  //Line: 06

4. You initialize $apps with an empty array:

   $apps = [];  //Line: 08

5. Now you loop through each element of $dependencies, which has 1 element here (["PredefinedApp"])

6. In the loop you do the following:

   6.1 Assign the return value of a function call to an array index:

   $apps[$name] = $dependName($this);  //Line: 10
   //$apps["App1"] = PredefinedApp($this);

7. You call the function:

   PredefinedApp(){ /* Code here */}  //Line: 28

8. Now you create again a new instance of: App in PredefinedApp() same as before (Point 2 - 6, expect in the constructor you have other variable values + you don't enter the loop, since the array is empty)

9. You assign a closure to a class property:

   $app->cleanup = function($parent){  //Line: 31
       //Do stuff like: echo "I'm saved";
   };

10. You return the new created object of App:

    return $app;  //Line: 34

11. Here already the __destruct() gets called, because when the function ends the refcount goes to 0 of that zval and __destruct() is triggered. But since $this->apps is empty nothing happens here.

12. The new created object in that function gets returned and assigned to the array index (Note: We are back from the function to point 6.1):

    $apps[$name] = $dependName($this);  //Line: 10
    //$apps["App1"] = PredefinedApp($this);

13. The constructor ends with assigning the local array to the class property:

    $this->apps = $apps;  //Line: 12

14. Now the entire script ends (We have done line: 37)! Which means for the object $app the __destruct() is triggered for the same reason as before for $app in the function PredefinedApp()

15. Which means you now loop through each element from $this->apps, which only holds the returned object of the function:

    public function __destruct(){  //Line: 15
        foreach($this->apps as $dep){
            $result = $dep->cleanup($this);
        }
    }

    Array(
        "App1" => App Object
            (
                [apps:protected] => Array
                    (
                    )
    
                [name] => PredefinedApp
                [cleanup] => Closure Object
                    (
                        [parameter] => Array
                            (
                                [$parent] => <required>
                            )
    
                    )
    
            )
    )
    

16. For each element (Here only 1) you execute:

    $result = $dep->cleanup($this);  //Line: 17

    But you don't call the closure! It tries to call a class method. So there is no cleanup class method, it's just a class property. Which then means __call() gets invoked:

    public function __call($name, $arguments){  //Line: 21
        if(is_callable([$this, $name])){
            return call_user_func_array([$this, $name], $arguments);
        }
    }

17. The $arguments contains itself ($this). And is_callable([$this, $name]) is TRUE, because cleanup is callable as closure.

18. So now we are getting in the endless stuff, because:

    return call_user_func_array([$this, $name], $arguments);  //Line: 23

    Is executed, which then looks something like this:

    return call_user_func_array([$this, "cleanup"], $this);

    Which then again tries to call cleanup as a method and again __call() is invoked and so on...

So at the end of the entire script the hole disaster starts. But I have some good news, as complicated this sounds, the solution is much simpler!

Solution

Just change:

return call_user_func_array([$this, $name], $arguments);  //Line: 23

with this:

return call_user_func_array($this->$name, $arguments);
                           //^^^^^^^^^^^ See here

Because by changing it like this you don't try to call a method, but the closure. So if you also put:

echo "I'm saved";

in the closure when you assign it, you will end up with the output:

I'm saved