On 3 May 2011 23:30, Sam Berlin <[email protected]> wrote: > > On Tue, May 3, 2011 at 6:14 PM, Stuart McCulloch <[email protected]>wrote: > >> Hi Sam, >> >> I think the change introduced in r1543 to fix Issue 626 breaks the >> semantics of InjectionListeners in certain circumstances. >> >> Previously when the InjectionListener was called you could be certain all >> @Inject annotated members had been injected and any circular proxies were >> initialized. Unfortunately the change in r1543 means that the instance could >> now still contain uninitialized provider proxies at the time >> the InjectionListener is called. This makes InjectionListeners more fragile >> and less useful, especially wrt basic lifecycle operations. >> >> I'm not quite sure how best to solve this - yes detecting cycles between >> providers can be useful, but typically people put providers in to try and >> break cyclic dependencies. So this change might even lead to situations >> where (due to legacy code) there's a cycle that would have been solved with >> providers but now can't be solved. But assuming we can't wind back the clock >> (or make this check optional), is there any chance we could add a new SPI >> listener that gets called at the end of "circularGet"? ie. after the call >> to "finishConstruction" like the listeners in ConstructorInjector? That way >> we'd still have a way to manage the instance after all proxies are >> initialized, but before it gets passed back to the caller. >> > > The change should be strictly improving upon situations that would have > previously failed. There were two failure scenarios before: > > 1) Two providers were circular dependent directly on each other. This > scenario would have resulted in a StackOverflowError as each provided is > recursively called to provide for the other. > > 2) A provider is involved in a circular dependency that involves a > non-provider binding. If the Provider was a singleton, this would have > resulted in Scopes.SINGLETON failing with a "re-entrant provider" failure. > If it wasn't a singleton, I'm not 100% sure what would have happened (but > it was likely broken). > > Would you be able to write a failing test case that uses > InjectionListeners? The circular provider checks are intended to fix actual > problems that we ran into at Google, and should result in all around safer > code. I'm not sure what stands out with the circular dependency detection > on Providers that wouldn't have already caused problems with Constructors, > since the two have basically the same detection/handling model. >
Sure, I can write up a failing test - the degenerate case is quite easy to recreate with a Provider cycle and basic InjectionListener, albeit not ideal code. Note the difference between the Constructor and Provider case is that the Constructor fires off its listeners after the proxies are initialized (ie. their delegates set) whereas with the Provider case there are no listeners to fire (and therefore no way to perform some last minute management). I might experiment with adding the new SPI listener in sisu-guice. (For now I've rolled this change back over there as it breaks our Plexus adapter - which I admit does push Guice to the edge wrt. lifecycle management because it has to support legacy code and semantics involving some very tortuous cycles!) sam > -- Cheers, Stuart -- You received this message because you are subscribed to the Google Groups "google-guice" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/google-guice?hl=en.
