-
Notifications
You must be signed in to change notification settings - Fork 38.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Circular references between custom scoped beans should be supported [SPR-2691] #7379
Comments
Mike Youngstrom commented I'm experiencing this problem to for regular REQUEST and SESSION scoped beans too. Should I break the request and session scoped beans problem out into a separate bug? |
Juergen Hoeller commented Let me clarify that circular references between non-singletons are not supported at present. For singleton beans, we do exactly such early registration of half-initialized bean instances to resolve circular references. The latter is one of the biggest source of bugs in the entire Spring BeanFactory... Circular references are generally discouraged and fragile, since initialization order starts to matter in this case. I refrain from doing such early exposure of bean instances in other scopes. This can lead to all sorts of unwanted side effects, for example with the propagation of session attributes in a cluster... I rather consider adding a check that throws a meaningful exception instead of an endless loop there. All of this applies to the "request " and "session" scopes as well as to any custom scopes. Juergen |
Juergen Hoeller commented FYI, I've added a check that throws a corresponding exception in case of such a circle - for all non-singletons, which includes prototypes as well as beans in special scopes. IMO, this is the most reasonable solution at this point. We can still consider supporting circular references for non-singletons, but that's a separate issue (and I strongly object to it). From my perspective, this issue is about not going into the endless loop. Hence, I intend to close it with that present solution unless there are strong objections... Juergen |
Piotr Kolaczkowski commented I think throwing an exception is a reasonable partial solution, but for me as a developer, much cleaner solution would be to make circular dependencies as a global configuration option. The developer could choose whether she wants to allow circular references for singletons / scoped beans and/or prototypes. The default could be "disabled". Allowing circular references for singletons only and not for the other types of beans lacks orthogonality. Spring should not force the developer to choose any particular solution over another. It is the developer's thing. I know that circular references are in most cases evil, but if the developer knows what he is doing, why not support them? By the way, scoped circular references work perfectly if used together with AOP-proxies. AOP proxies break the endless loop by making initialization VERY LAZY. :D This is another orthogonality break. Another lack of orthogonality in scoped beans is that there is no possibility to eager-initialize them when the new scope context is created (e.g a new HTTP session). It would be nice to have such feature. This could be implemented simply by registering another callback in the Scope interface. The one for destruction is already there ;) |
Juergen Hoeller commented Keeping this issue open as an enhancement request, targetting the Spring 2.1 timeframe. #7455 is considered resolved in that an exception is now thrown as an endless loop. Regarding the general sentiment, I rather feel like removing circular dependency support even for singletons. No worries, we're not gonna do this ;-) We just received so many issues reports in that area that we strongly recommend against circular references in the meantime. What's the general problem with circular references? It's that, by definition, one of the two involved beans receives a reference to the other without that other bean being fully initialized yet. If the receiving setter method is a non-trivial implementation (that is, doesn't just store the reference), this can lead to unexpected NPEs, or even worse, wrong configuration decisions based on the incomplete state. This can remain completely unnoticed, since the system might be in a workable state - just not in the expected one. This is also why lazy-initializing AOP proxies for the target beans can solve the problem: They just prepare proxy references, which clarifies the exposed signatures and does not lead to intialization of the target beans until actually accessed. In particular, simply storing the reference does not necessarily lead to initialization of the corresponding target bean, in contrast to a circular reference that involves plain POJO instances. Furthermore, our Scope interface allows for atomic fetch-and-create-if-not-present operations through its "get" method with an ObjectFactory argument. This would conflict with circular reference support, where the ObjectFactory would have to expose an early instance in the middle of the creation process. This is very undesirable if not even unacceptable in distributed scopes such as a clustered session or a clustered cache. All in all, circular references between singletons are kind-of acceptable since, at least, they typically get resolved on application startup and show their effect right away. Circular references between other scopes would be more fragile, since the exact initialization procedure would depend on which bean gets accessed first at runtime. And if at all, this would have to be done with the concrete target scope explicitly expressing that it is capable of such circular reference support in the first place. Juergen |
Antranig Basman commented Juergen - I seriously would invite you to consider forbidding circular dependencies though :P Although as you say in a static context the issues are seen early, there is still the serious issue that they occur inconsistently and in a way perturbed by container startup order. When dealing with a very large environment where startup can take several minutes, and where the container definition is built up by contributions from many different teams, the issues can be extremely obscure and time-consuming to track down. Could I second Piotr's request for "much cleaner solution would be to make circular dependencies as a global configuration option. The developer could choose whether she wants to allow circular references for singletons / scoped beans and/or prototypes. The default could be "disabled""! People should be directed in the first instance to the AOP lazy-init solution if they want circles, and then perhaps to this global configuration option if they are feeling truly lazy and feel like having an incoherent architecture :P The current flag in #6186 is very hard to access without intruding enormously on the configuration. Ref my older JIRA of #6741. |
Juergen Hoeller commented We do not plan to allow for circular references between non-singletons. We'll rather make it easier to disalllow circular references as suggested in #6741. Juergen |
Piotr Kolaczkowski opened SPR-2691 and commented
Hi. I created a custom scope by overriding the get method in such a way:
public Object get(String name, ObjectFactory factory) {
if (name was associated with the current thread)
return object associated with the current thread;
Object result = factory.getObject();
associate result with the current thread;
return result;
}
I registered scope programatically and it works as expected until I do a circular reference between two beans using this scope:
<bean name="bean1" class="foo.Foo" scope="myScope">
<property name="bean2" ref="bean2"/>
</bean>
<bean name="bean2" class="bar.Bar" scope="myScope">
<property name="bean1" ref="bean1"/>
</bean>
Spring falls into an endless loop creating bean1, bean2, bean1, bean2, ....
If I change any of scopes to "singleton", it creates each bean at most 2 times (or once if both scopes are singleton).
I think the problem is that while the factory.getObject() is called, the object is NOT YET registered within the given scope, so the next call to the Scope.get(...) from the referenced bean enters factory.getObject() once again, closing the loop. The next call should have returned the already created bean, but wait.... it cannot be returned because we haven't returned from the factory.getObject() yet... So it is the chicken and egg problem.
I suggest providing 2-step bean instantiation: 1st instantiate the bean using constructor injection, and in 2nd step complete the bean by launching the setter injection. Then it could be possible to
associate the object with the scope (e.g. current thread) between those steps.
By the way is there any reasonable work-around other than removing the circular reference?
Affects: 2.0 final
Issue Links:
1 votes, 1 watchers
The text was updated successfully, but these errors were encountered: