bineanzhou / google-guice

Automatically exported from code.google.com/p/google-guice
Apache License 2.0
0 stars 0 forks source link

Formalize semantics of circular-dependency proxies #220

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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 reported on code.google.com by limpbizkit on 17 Jul 2008 at 8:10

GoogleCodeExporter commented 9 years ago
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().

Original comment by crazybob...@gmail.com on 17 Jul 2008 at 8:23

GoogleCodeExporter commented 9 years ago
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.

Original comment by crazybob...@gmail.com on 17 Jul 2008 at 8:27

GoogleCodeExporter commented 9 years ago
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.

Original comment by robbie.v...@gmail.com on 21 Jul 2008 at 7:28

GoogleCodeExporter commented 9 years ago
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.

Original comment by crazybob...@gmail.com on 7 Aug 2008 at 12:52

GoogleCodeExporter commented 9 years ago

Original comment by limpbizkit on 18 Nov 2008 at 7:33

GoogleCodeExporter commented 9 years ago
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).

Original comment by sberlin on 14 Apr 2010 at 1:23

Attachments:

GoogleCodeExporter commented 9 years ago
InjectorBuilder.disableCircularProxies committed in r1151.

Original comment by sberlin on 24 Apr 2010 at 9:43

GoogleCodeExporter commented 9 years ago

Original comment by sberlin on 22 Feb 2011 at 1:52

GoogleCodeExporter commented 9 years ago
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.

Original comment by en...@stolsvik.com on 22 Feb 2011 at 3:57

GoogleCodeExporter commented 9 years ago
Related to issue #296.

Original comment by en...@stolsvik.com on 22 Feb 2011 at 4:00

GoogleCodeExporter commented 9 years ago
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.

Original comment by tpeie...@gmail.com on 22 Feb 2011 at 4:23

GoogleCodeExporter commented 9 years ago
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!

Original comment by sberlin on 23 Feb 2011 at 1:45

GoogleCodeExporter commented 9 years ago

Original comment by cgruber@google.com on 18 Nov 2013 at 9:10