ThreadLocal leaks

[Niclas Hedhman <ni...@hedhman.org>]

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

Re: ThreadLocal leaks

[Paul Merlin <pa...@nosphere.org>]

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?