Danilo, I implemented the wrapper you described here: http://mail-archives.apache.org/mod_mbox/db-ojb-user/200506.mbox/[EMAIL PROTECTED]
This didn't clean up any of the objects I have left over after my application shuts down. After the application is stopped in Tomcat, the wrapper class is pointed to by PersistenceBrokerFactoryFactory.singleton which never gets null'd out (and other static variables still hold references such as MetadataManager.singleton). I guess I'd like to see a shutdown() or destroy() method at some level that would clear out all of these static references. Bob -----Original Message----- From: Danilo Tommasina [mailto:[EMAIL PROTECTED] Sent: Friday, July 15, 2005 3:33 AM To: OJB Users List Subject: Re: Cleaning up OJB when shutting down an application Hi Armin, Below you can find a possible patch for the PersistenceBrokerThreadMapping, this also contains a shutdown() method to guarantee that the PersistenceBrokerThreadMapping is cleaned up correctly even if PersistenceBrokers are not closed correctly. I would also suggest adding a central shutdown() method (where?) that should call: - PersistenceBrokerFactory.releaseAllInstances() - ConnectionFactoryFactory.getInstance().createConnectionFactory().releaseAllResources() - The new call to PersistenceBrokerThreadMapping.shutdown() We are also executing following calls, these don't seem to be necessary if the 'evil ThreadLocal memory leak' is solved, include them at your choice. We were using these to reduce the amount of leaked memory before applying the workaround. Note that some more checks may be necessary if the shutdown is called on an uninitialised OJB instance (errors in the config, connection failure, ...) PersistenceBroker pb = null; try { pb = PersistenceBrokerFactory.defaultPersistenceBroker(); pb.clearCache(); // Clear complete OJB cache } catch ( PersistenceBrokerException pbe ) { // Silently ignore, this error should be handled somewhere else (initialsiation of OJB failed } finally { if ( pb != null ) { pb.close(); } } And finally MetadataManager.getInstance().removeAllProfiles() ---------------------- patched PersistenceBrokerThreadMapping, I also allowed myself to add some missing Javadoc ;) ---------------------- public class PersistenceBrokerThreadMapping { /** A Collection of all HashMaps added to the <CODE>ThreadLocal currentBrokerMap</CODE> that are still -alive-. * If all the PersistenceBrokers are always correctly closed, the Collection should remain empty. * The Collection is iterated trough when calling the <CODE>shutdown()</CODE> method and all the maps that are * still alive will be cleared. */ private static Collection loadedHMs = new HashSet(); // NEW /** * The hashmap that maps PBKeys to current brokers for the thread */ private static ThreadLocal currentBrokerMap = new ThreadLocal(); /** Mark a PersistenceBroker as preferred choice for current Thread * @param key The PBKey the broker is associated to * @param broker The PersistenceBroker to mark as current */ public static void setCurrentPersistenceBroker(PBKey key, PersistenceBroker broker) throws PBFactoryException { HashMap map = (HashMap) currentBrokerMap.get(); WeakHashMap set = null; if (map == null) { map = new HashMap(); currentBrokerMap.set(map); loadedHMs.add( map ); // NEW } else { set = (WeakHashMap) map.get(key); } if (set == null) { // We emulate weak HashSet using WeakHashMap set = new WeakHashMap(); map.put(key, set); } set.put(broker, null); } /** Unmark a PersistenceBroker as preferred choice for current Thread * @param key The PBKey the broker is associated to * @param broker The PersistenceBroker to unmark */ public static void unsetCurrentPersistenceBroker(PBKey key, PersistenceBroker broker) throws PBFactoryException { HashMap map = (HashMap) currentBrokerMap.get(); WeakHashMap set = null; if (map != null) { set = (WeakHashMap) map.get(key); if (set != null) { set.remove(broker); if ( set.isEmpty() ) { // NEW map.remove( key ); // NEW } // NEW } if ( map.isEmpty() ) { // NEW currentBrokerMap.set( null ); // NEW loadedHMs.remove( map ); // NEW } // NEW } } /** * Return the current open [EMAIL PROTECTED] org.apache.ojb.broker.PersistenceBroker} * instance for the given [EMAIL PROTECTED] org.apache.ojb.broker.PBKey}, if any. * @param key * @return null if no open [EMAIL PROTECTED] org.apache.ojb.broker.PersistenceBroker} found. */ public static PersistenceBroker currentPersistenceBroker(PBKey key) throws PBFactoryException, PersistenceBrokerException { HashMap map = (HashMap) currentBrokerMap.get(); WeakHashMap set; PersistenceBroker broker = null; if (map == null) { return null; } set = (WeakHashMap) map.get(key); if (set == null) { return null; } // seek for an open broker, preferably in transaction for (Iterator it = set.keySet().iterator(); it.hasNext(); ) { PersistenceBroker tmp = (PersistenceBroker) it.next(); if (tmp == null || tmp.isClosed()) { it.remove(); continue; } broker = tmp; if (tmp.isInTransaction()) { break; // the best choice found } } return broker; } /** Clean up static fields and any registered ThreadLocal contents to grant a clean * shutdown/reload of OJB within hot-deployable applications. */ public static void shutdown() { for ( Iterator it = loadedHMs.iterator(); it.hasNext(); ) { ((HashMap) it.next()).clear(); } loadedHMs.clear(); loadedHMs = null; currentBrokerMap = null; } } > Hi Danilo, Bob, > > AFAIK there was no patch checked in conjunction with PBThreadMapping. > As you said the threads in tomcat will be recycled and it seems that > ThreadLocal will cause a memory leak in this case: > http://www.jroller.com/page/tackline/20050310 > (sounds similar, but maybe I'm wrong) > > So we need some kind of shutdown method in PBF and in PBThreadMapping. > Could you both summarize the needed changes I will check in a patch. I > found the patch for PBThreadMapping (the new "shutdown"-method). > What's needed too? > > regards, > Armin > > Danilo Tommasina wrote: > >> Hello, >> >> please see my post 'ThreadLocal causing memory leak' of 9. Jun. 2005, >> this is the same problem described there. I posted some code that can >> be used as temporary solution or workaround and discussed a bit about >> the issue with Martin Kalén, however I do not know the current >> status... >> >> Both the workaround in the PersistenceBrokerFactoryDefaultImpl and my >> proposed 'clean' solution in the PersistenceBrokerThreadMapping (I >> sent the definitive code directly to Martin) seems to work correctly >> in our application. To note is that we are using only the PB API and >> are not using proxies. The workaround in the >> PersistenceBrokerFactoryDefaultImpl 'may' cause problems or >> performance issues with proxies, however I cannot test it, I did just >> a short and not very deep analysis of the code searching for possible >> side effects. >> >> As I cannot currently access the apache CVS (it seems to be flooded >> and it is currently down), I cannot say if the issue has already been >> patched in the latest CVS version or not. >> >> bye >> Danilo >> >> Glugla, Robert T wrote: >> >>> 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. >>> >>> Any suggestions? >>> >>> Thanks. >>> >>> Bob Glugla >>> Boeing >>> >>> -------------------------------------------------------------------- >>> - >>> To unsubscribe, e-mail: [EMAIL PROTECTED] >>> For additional commands, e-mail: [EMAIL PROTECTED] >>> >> >> [EMAIL PROTECTED] >> www.risksys.com >> >> --------------------------------------------------------------------- >> 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] > -- Danilo Tommasina, Dipl. Ing. FH Telecom Software Engineer, Chief Software Architect RCS Riskmanagement Concepts Systems AG Technoparkstrasse 1 CH-8005 Zuerich T: +41 1 445 29 08 [EMAIL PROTECTED] www.risksys.com --------------------------------------------------------------------- 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]
