Framework: Cyclic Dependency Check in IoC

[engma]

when ever I have e cyclic depency in Laravel, I can barely know that it is the issue I think an appropriate exception could be thrown to illustrate were the cycle is found, and suggest some break ups.
I know there are many ways to fix this cyclic issue, but thrown an exception is a huge help to just pin-point that there is one.

is there a reason why Laravel up till now don't handle this issue ?

[GrahamCampbell]

How would we implement this?

[etcinit]

A naive solution could be to keep track of the size of the $buildStack when resolving something out of the container and then enforce some ridiculous artificial upper limit. However, without analyzing the content of the dependency stack, any exception thrown would just indicate that there is a cycle but not explain why it is happening.

[engma]

I implemented this once using:

• A Waiting List array, this is an assiciative array with hashes of class names as keys and full namespaced name as values example: `[123456=>'AppHttpMyController']
• Then everytime a class requests a dependancy, I check if that dependancy is in the waiting list, if its in the waiting, I throw an exception, showing the two classes in the cycle.

I think this can be extended to include full cycles, but I haven't dug that deep in the methodology

[etcinit]

Actually, never mind the thing about $buildStack. A fatal error occurs when there is a cyclic dependency: Maximum function nesting level of '100' reached, aborting!

[engma]

@etcinit I know yes, but it doesn't show where the issue is formed, I was just thinking maybe we can take this a step further, but showing a detected cycle.

[etcinit]

and maybe it is only checked when APP_DEBUG is true so that it doesn't affect performance for production applications

[engma]

@etcinit Exactly, it should be a configuration option, that you can turn it on and off, but when I dug into why this "limit exceeded" is thrown, it was doing a recursive function call. I think this is the real performance issue, and it would really help us understanding the nature of the issue.
And actually you may get a "limit exceeded" Exception, with any thing that is self recurring.

[etcinit]

There seems to be algorithms for detecting these kind of situations (like https://en.wikipedia.org/wiki/Tarjan%27s_strongly_connected_components_algorithm) but it seems a bit overkill for a web framework.

[engma]

I think those are for full cycle detection, I used the proposed solution of the Waiting List, it is effecient just to break the cycle and show the class that had rounded cycles, maybe we can just provide this small detection. and enhance it later

[GrahamCampbell]

Maximum function nesting level of '100' reached, aborting!

That limit would be way to low for us to impose with that build stack count. Also, note the new xdebug default in the 2.3 release will be 250. I already have applications that go over 100 nested function calls.

[engma]

@GrahamCampbell so we may just need to implement the suggested algorithm, or another simple solution,
I know its a web framework, but since it depends heavily on IoC, then IMHO I think that Cyclic Dependency checks are necessary.

[GrahamCampbell]

Ok. Are there any other php dependency containers implementing a cyclic dependency prevention algo?

[engma]

@GrahamCampbell I did a small search and could find this project https://github.com/antimax/PHP-DependencyInjectionContainer, I didn't really dig deep in it, but perhaps there are other frameworks.
Also Symphony has a ServiceCircularReferenceException in its DI container have a look at
this.

Those are the ones I could find so far.

[ibrasho]

@GrahamCampbell @engma Symfony DI component is checked when its compiled. I don't know/think it does the check when registering service at runtime.

Check https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DependencyInjection/Compiler/CheckCircularReferencesPass.php , which shows how it's done when compiling the dependency graph.

We could implement some checks to ensure the same path is not repeated.
I think this should be a debug-only feature since it will create overhead for fetching services from the container.

[GrahamCampbell]

I disagree about running it in debug mode only. The overhead would be comparatively small.

[engma]

@ibrasho I think, it won't make a big overhead, and with the deferred services in Laravel, you can limit the services loaded for each request and this way limit the overhead even further.
I will try to implement a simple implementation in the next few days, after I deliver my current task.

[ibrasho]

I made something simple that seems to work.

I think the exception message could be better. What do you think?

[neomerx]

:-1: This is development cycle issue which is easily covered by a trivial test. Maximum function nesting level of '100' reached, aborting! is totally fine and you will see root problem 100 times in the stack trace.

[ibnujakaria]

Wondering is there any update on this issue.

[renepardon]

Nothing going on here?