Ok, will do. I looked at where to put the try/catch earlier and figure that the try block should start just before line 686 and end just after line 695, which would encompass the code where Di::$currentDependencies is manipulated. The catch block would then reset Di::$currentDependencies to array() and then re-throw the exception, so on entry the next time Di::$currentDependencies would be clean.
Chris On Mon, Mar 25, 2013 at 8:33 PM, Marco Pivetta <[email protected]> wrote: > Hi Chris, > > Can you please report this on the issue tracker at > https://github.com/zendframework/zf2/issues ? > > Anyway, where should the try-catch be placed? Because there's a lot of > entry points that could cause this as far as I know. > > Marco Pivetta > > http://twitter.com/Ocramius > > http://ocramius.github.com/ > > > On 26 March 2013 03:31, Chris Toomey <[email protected]> wrote: > >> We're seeing occasional CircularDependencyExceptions being reported when >> using Dependency Injection. By spurious I mean that 1 out of every say >> 10,000 requests to get() an instance of a given class will fail with that >> exception being thrown, while the rest of them will work fine. >> >> I dug into the code today to see how that could happen, and found that >> it's >> triggered by 1) using the DI container to get an instance of some class C, >> during which in one of the recursive calls to Di->get() or >> Di->newInstance() some dependent class D's instantiator throws an >> exception, and 2) doing a subsequent Di->get() for D, C, or any other >> class >> that depends on D. >> >> The cause is that in case of (1), proper cleanup is not done on the >> Di::$currentDependencies variable when an exception is thrown, and thus >> it's left in an unclean state (with some dependencies vs. empty), so that >> during (2), Di->resolveMethodParameters() spuriously thinks there's a >> circular dependency. >> >> The fix would be to put a try/catch block around the code that pushes/pops >> Di::$currentDependencies and does recursive calls, and in the catch block >> reset Di::$currentDependencies before re-throwing the exception. Maybe >> there's other cleanup that should be done there too, but this is the one >> we've been bitten by. >> >> thx, >> Chris >> > >
