Formalize semantics of circular-dependency proxies

[gissuebot]

From limpbizkit on July 17, 2008 16:10:12

Currently what goes on to make circular dependencies work is a little
magical. For example, what methods you can call on an object that was
@Injected in a constructor.

Original issue: http://code.google.com/p/google-guice/issues/detail?id=220

[gissuebot]

From crazyboblee on July 17, 2008 13:23:41

One idea: a circular dependency could be an error by default, and then you could
insert an @Proxy annotation or something to tell Guice you want that interface proxied.

Of course the core problem remains: we can't always detect circular deps until
runtime. It would be nice if we could just crawl the graph, but the constructor could
still call Injector.getInstance() or Provider.get().

[gissuebot]

From crazyboblee on July 17, 2008 13:27:46

I suppose this is also a good place to note that moving the dep to a setter probably
won't fix anything. The problem is that we inject fields and and methods immediately
after the constructor (i.e. before injecting that object into other constructors).
This means that when it comes time to call the setter, we still don't have the other
instance yet.

In 2.0, we should consider injecting all constructors first and then going back and
injecting all fields and methods in a second stage.

Moving the dep to a setter is my favorite way to break up circular dependencies
because it means the code will still work find outside of Guice.

[gissuebot]

From robbie.vanbrabant on July 21, 2008 12:28:46

Injecting constructors and members in a different stage might be a good way to solve
the technical issue with circular dependencies, but I do think it would break a lot
of apps. In my Factory days I used to return instances that were fully configured,
setters included. Not having these values when constructing other objects could be
annoying.

If all Guice users follow all the good practices like immutability, not doing much
work in constructors, removing indirection etc. then we are fine. But the world isn't
perfect, and runs on tons of legacy code.

Maybe there are also other concerns, like what to do with eager singletons? Don't know.

[gissuebot]

From crazyboblee on August 06, 2008 17:52:53

In 2.0, we need to at least try to break injection into two stages so we inject all
constructors first, and then all members in a second stage. I'm sure there will be
some fuzziness here--a constructor can force members to be injected and a member can
trigger additional constructor injections.

Labels: Milestone-Release2.0

[gissuebot]

From limpbizkit on November 17, 2008 23:33:52

(No comment was entered for this change.)

Labels: -Milestone-Release2.0 Milestone-Release2.1

[gissuebot]

From sberlin on April 14, 2010 06:23:04

Not formalizing semantics, but here's a small patch to add a disableCircularProxies
option to InjectorBuilder, which is a step in the direction.  If folks see utility in
this, I'll commit it -- but want to get some reaction & feedback first.  I can see an
argument for not adding it until circular detection is fail-fast (ie, at Injection
creation time instead of instance creation time).

Attachment: gist
   circular-patch.txt

[gissuebot]

From sberlin on April 24, 2010 14:43:11

InjectorBuilder.disableCircularProxies committed in r1151 .

[gissuebot]

From sberlin on February 21, 2011 17:52:06

(No comment was entered for this change.)

Labels: -Milestone-Release2.1

[gissuebot]

From endre@stolsvik.com on February 22, 2011 07:57:57

Would it not be possible to break injection into two phases, as Bob points out in comment#4?

Comment#3 states: "Injecting constructors and members in a different stage might be a good way to solve the technical issue with circular dependencies, but I do think it would break a lot of apps. In my Factory days I used to return instances that were fully configured, setters included. Not having these values when constructing other objects could be annoying."

This is not a correct assessment, as I understand it: If some object A /depends/ on both constructor AND setters having been run on object B when A gets B (when A is being constructed or some setter run), then Guice will break that app as it is now: This because the dependencies that B have gotten /might/ have been proxies that aren't "filled" yet (or it might not, you just never know).

It should be pretty simple to construct a graph of which objects should be constructed in which order (first the ones w/o anything in the constructor, then the ones only taking from this pool, then ..). If it is not possible to do this construction w/o proxies, then fail. Then afterwards, do the setting.

[gissuebot]

From endre@stolsvik.com on February 22, 2011 08:00:36

Related to issue #296 .

[gissuebot]

From tpeierls on February 22, 2011 08:23:38

Whatever you do, don't lost track of the requirement that there be a happens-before edge between the construction/injection of an object and its provision. (Without it, objects can get published unsafely.)

Circular dependencies in general make it impossible to meet that requirement. If there are clever ways to deal with specific cases without violating the requirement, great, but it's better to fail early and obviously than risk unsafe publication.

[gissuebot]

From sberlin on February 22, 2011 17:45:15

If anyone is able to code up & contribute a patch that can create the object graph & order the way Guice constructs objects, it would be very much appreciated!

[gissuebot]

From cgruber@google.com on November 18, 2013 13:10:57

(No comment was entered for this change.)

Status: Acknowledged