[DependencyInjection] Adding an exception on circular reference #28452
Conversation
72576c9 to
877b660
Compare
877b660 to
4b27065
Compare
| @@ -1017,6 +1018,10 @@ public function setDefinition($id, Definition $definition) | |||
| throw new BadMethodCallException('Adding definition to a compiled container is not allowed'); | |||
| } | |||
|
|
|||
| if ($definition instanceof ChildDefinition && $id === $definition->getParent()) { | |||
There was a problem hiding this comment.
I would suggest moving the logic to ResolveChildDefinitionsPass.
Also, can't the loop involve intermediary ids? A => B => C => A?
There was a problem hiding this comment.
Thanks for your feedback.
I can move it to ResolveInstanceofConditionalsPass (here), but I think it's not a good option too...
Currently I can't find the way to move it to ResolveChildDefinitionsPass. If you have an idea...
| * | ||
| * @author Nicolas LEFEVRE <weblefevre@gmail.com> | ||
| */ | ||
| class ArgumentCircularReferenceException extends RuntimeException |
There was a problem hiding this comment.
if we want this to make it into a bug fix release, we should not add any new class in the PR (otherwise that'd be a smell for a new feature).
Instead, what about throwing a ServiceCircularReferenceException?
There was a problem hiding this comment.
OK, throwing ServiceCircularReferenceException should be a good idea...
|
Closing in favor of #28480, which is closer to the final patch. |
When a
ChildDefinitionreferences itself as parent (or as ancestor in case of longer cycle), an infinite loop happens in the resolution logic.It's a try to fix this "bug", thank's for your feedback...