Hi Danilo,
> To Armin:
> Could you please also look if it is possible to add shutdown methods to
> stop the eviction Threads of the broker and (if used) the connection
pool?
> These methods should then be internally called by the 'central shutdown
> method'.
>
I had a first look at commons-pool source-code (version 1.2, latest from
CVS is 1.3 - maybe things changed). It's not possible to directly stop
the eviction thread (a daemon thread). There are no (public) methods to
stop the thread. Internal a cancel()-method exists to stop the thread.
But when the pool is closed (ObjectPool#close()) the eviction thread
will be stopped too. Currently OJB call ObjectPool#clear() when method
#releaseAllInstances() is called, but ObjectPool#clear() doesn't stop
the eviction thread.
So we can fix this when using ObjectPool#close() instead of
ObjectPool#close().
regards,
Armin
Danilo Tommasina wrote:
Hi Thomas, Hi Armin,
thanks for looking into this.
From the link (http://www.jroller.com/page/tackline/20050310) that
Armin posed about the problems with ThreadLocal.
----
From 1.4, each Thread has a map from ThreadLocal to a local value. As
only a single thread ever uses each map it can be fast. The mapping uses
a form of weak hash map with strong references to the local values. Any
strong reference path from the local value to the ThreadLocal protects
the ThreadLocal, and hence the value, from garbage collection. *The dead
values cannot therefore be collected until the thread exits*.
----
The last phrase above is the crucial part. This is exactly the problem,
the Threads remain alive for the whole server lifecyle preventing a
cleanup of OJB when just redeploying/unloading a web-application.
Using ThreadLocal is ok, however you have to guarantee that the mapped
variables are removed from the ThreadLocal context when they are no
longer used.
This is what my proposed patch to the PersistenceBrokerThreadMapping
does. If the user correctly closes the PersistenceBrokers then there is
also no need for the shutdown method, the shutdown method I added is
just there to guarantee that all ThreadLocal variables are released.
To the solution proposed by Thomas:
Using the session or the servlet context instead of ThreadLocal may be
seen as a bad programming pattern since you will have to expose typical
presentation-tier objects down into the persistence-tier and get them
all carried over trough the business-tier. OUCH!
And before somebody comes up with the idea... NO!!! For a large and well
designed application, using the PersistenceBroker stuff at JSP level is
not a good design pattern! ;)
To Armin:
Could you please also look if it is possible to add shutdown methods to
stop the eviction Threads of the broker and (if used) the connection pool?
These methods should then be internally called by the 'central shutdown
method'.
----
Please note that my proposed patch to the PersistenceBrokerThreadMapping
works, our application can now cleanly shutdown.
However I saw that there are some other classes that use ThreadLocal
variables, they are used only under special combinations of the
configuration, so fixing also those ones may not be a hi-prio issue. I
think that a similar approach with a shutdown method will also work in
those classes.
Should I add a new issue into JIRA adding also the proposed bug-fix?
cheers
Danilo
Thomas Dudziak wrote:
On 7/21/05, Danilo Tommasina <[EMAIL PROTECTED]> wrote:
Hi there
are there any news about this issue? do you need more information?
Robert? did you find out if you have pool/connection eviction threads
running?
you can disable automaitc eviction by setting in OJB.properties:
timeBetweenEvictionRunsMillis=-1
and by setting the attribute timeBetweenEvictionRunsMillis="-1" in
your connection-pool settings in the repository.xml
Please note that somebody else reported that my dirty fix with the
override of the PersistenceBrokerFactoryDefaultImpl works for them
too, so I guess that you
have some other problem in your code, Robert. (see the followups of
the 'ThreadLocal causing memory leak' post)
Here a little summary why from my point of view, the use of
ThreadLocal is causing the leak.
Tomcat (and probably other web-servers) seem to have a pool of Thread
objects that are always alive and ready to serve http requests. The
Threads sit idle until
a http request comes in, when the request is served the thread is
returned to the pool and sits idle again.
Now the problem is that these Thread instances are class-loaded and
created by the server's main ClassLoader, if OJB puts instances of
any kind in a ThreadLocal
variable without removing them, a link between classes/objects
loaded/created in the main ClassLoader and the web-application's
ClassLoader is created.
These links will remain active and keep the web-app ClassLoader
active until the Thread instaces are garbage collected. However since
these Thread instaces are
never released the links to the web-app class-loaded objects will
remain alive and prevent the web-app ClassLoader to be garbage
collected after unloading the
web-application. If the web-app ClassLoader is not garbage collected
then no static references (singletons, constants,...) will be cleaned
and no classes will
be unloaded.
Cleaning static references 'manually' will allow garbage collection
will free heap space, but will break system design (a singleton
should be a final static
object and not a static one) and more important it will not allow the
web-app ClassLoader to be garbage collected with all the Class
objects that have been
loaded. This will lead over time to a even more problematic
OutOfMemoryError in the Perm. Gen. space.
It is quite a chain reaction that costed me long time to figure
out... :(
if you need more info just ask ;)
if you think that my analysis of the problem is crap, just say so, I
am open to discussion ;)
bye
Danilo
Environment:
OJB: 1.0.3
JProfiler: 4.0.1
Tomcat: 5.0.19
JVM: Sun 1.4.2_06-b03
While profiling our OJB application, we noticed that after we shut it
down in Tomcat, we had plenty of org.apache.ojb.broker.* and
org.apache.ojb.metadata.* objects left behind in memory. Using
JProfiler
we tracked it down to several static variables not getting cleared out
including:
PersistenceBrokerFactoryFactory.singleton
MetadataManager.singleton
...several others...
Does anyone have any suggestions as to how to clean these up
properly? I
looked for destroy methods and couldn't find any. As a test, I
downloaded the OJB source and added destroy methods to
PersistenceBrokerFactoryFactory & MetadataManager that I called from a
context listener and it worked ok. Really, really don't want to do
this
myself. I'm guessing I'm just overlooking some easy way for cleaning
these up already provided in OJB.
ThreadLocal is considered to be a risk in web applications because of
these reasons. So perhaps it might be useful to make that part
configurable in OJB. Eg. in web apps OJB could use the ServletContext
or the session instead of ThreadLocal to store away data.
If there is interest in this, could you perhaps create an issue in
JIRA ? I'll have a look into it then.
Tom
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
