Niclas Hedhman a écrit : > I just read an interesting article[1], which highlighted something that > never really occurred to me. > > And I think we are guilty of not clearing up the ThreadLocals upon > application passivation, and hence leaking massively. > > This mail is from my mobile, primarily as placeholder for myself, to create > a Jira issue about this. > > [1] > http://java.jiderhamn.se/2012/01/29/classloader-leaks-iv-threadlocal-dangers-and-why-threadglobal-may-have-been-a-more-appropriate-name/ > > Cheers Good read, thanks!
In core, we do have ThreadLocals in UnitOfWorkInstance and util.Dates. Only the former is of interest: `ThreadLocal<Stack<UnitOfWorkInstance>> current`. It is effectively never removed. The classloader problem aside, if the application behave correctly wrt. closing all its UnitOfWorks, the leak is limited to an empty Stack per touched thread, each backed by a small empty array most of the time. This makes me think that we do check for dangling UoWs after some tests, we should add such a check at passivation and display a reasonable warning to users in any case (ZEST-139). About the classloader problem, simple usecases (eg. start jvm, start app, stop app, stop jvm) are not affected. Stating the obvious here. On the other hand, live-reload development setups that create/dump numerous instances of an Application are affected. I never faced an OOM due to Zest in this scenario, but this is just some observation, no scrutiny, and should also be balanced by the fact that the apps were pretty lean. It could be a concern for some production setup that creates a lot of "short"-lived Application instances. I remember Tasos working on something like that. How things would behave then? Both the extent and the solution to the problem depend on the application threading-model. We could expose some api for users to integrate (eg. in a servlet filter) and do so in libraries where its needed. Or .. UnitOfWorkInstance could remove the ThreadLocal as soon as the Stack is empty and recreate it when needed but this may requires some extra synchronization. What do you think?
