[5.7] Track abstracts being resolved, fire resolving events once

[tomlankhorst]

Fixes #23699 by tracking abstracts that are currently being resolved.
If an object is built that is an instanceof of an object that is already on the stack, firing the resolving() callbacks is inhibited.
These callbacks are fired eventually when the instanceof object is resolved.

[tomlankhorst]

Put this on master, btw, because it changes (expected?) behavior and that might break existing apps.

[sisve]

1. There's a missing array_pop in the resolve() method for the early return.
2. These changes will still trigger the resolving-event for the class twice.

interface LoggerInterface {}

class LoggerImpl implements LoggerInterface {
    function __construct() {
        dump('logger constructor');
    }
}

class DepA {
    function __construct(LoggerInterface $logger) {}
}

class DepB {
    function __construct(LoggerInterface $logger) {}
}

class Service {
    function __construct(DepA $a, DepB $b, LoggerInterface $logger) {}
}

app()->singleton(LoggerInterface::class, LoggerImpl::class);
app()->resolving(LoggerInterface::class, function() {
    dump('resolving iface');
});
app()->resolving(LoggerImpl::class, function() {
    dump('resolving class');
});
app()->afterResolving(LoggerInterface::class, function() {
    dump('resolved iface');
});
app()->afterResolving(LoggerImpl::class, function() {
    dump('resolved class');
});
$service = app(Service::class);

The output is ...

"logger constructor" // separate weirdness, shouldn't this be between resolving and resolved?
"resolving iface" // removed by this pr
"resolving class" // still triggered with this pr
"resolved iface" // removed by this pr
"resolved class" // still triggered with this pr
"resolving iface"
"resolving class"
"resolved iface"
"resolved class"

The fact that the resolving event is triggered after the class has resolved is weird. I didn't expect that all.

[tomlankhorst]

You're totally right on the missing array_pop.
The resolving and afterResolving callbacks are all called in fireResolvingCallbacks(), after the construction of the object, before injecting it in the depending class' constructor.

Looking into your second remark...

[tomlankhorst]

I added a method that checks whether an object or a base class / interface is pending in the resolveStack. Then, the callback will skip and it will fire eventually when the last pending resolve call finishes.

So, for a Bar class that implements IFoo, that looks like this:

• app( IFoo::class ) resolves IFoo::class
• IFoo::class is bound to Bar::class
• app( IFoo::class ) resolves Bar::class
  • Bar::class is resolved: object is an instance of Bar
  • Bar's resolving callback is inhibited because IFoo - of which Bar is an instanceof - is in the resolveStack
• IFoo resolves
• IFoo triggers callbacks on the resolved object, an instance of IBar, and triggers resolving callbacks both on IFoo and Bar

[tomlankhorst]

@sisve I've added the situation you describe to the test.

[laurencei]

Before this is merged - can you run some scripts that creates like 10,000 Eloquent objects? What is the memory usage before/after this patch? And resolve loads of classes - what does that do?

The reason is - using arrays to track things was a cause to a bug I fixed a while ago in the container back in 5.4 here: #18812

I'm not saying your approach wont work - it probably does from the quick glance I did. But you gotta be very careful with arrays and the container and memory usage.

[tomlankhorst]

@laurencei Thanks for your comment. The $resolveStack is tracking an array of abstracts which are string types. Furthermore, the stack tracks currently resolving abstracts, not all abstracts.
So, the memory usage of this stack would have a maximum that scales linearly with the deepest dependency level in the application, which I expect would be rarely that great.

Simplified demo: https://3v4l.org/cEoe4

[taylorotwell]

This loop at every step of resolution is a little concerning. Could be a performance problem.

[tomlankhorst]

I'll assess the performance impact.

[taylorotwell]

Closing pending inactivity on performance reports.