Author: epbernard Date: 2010-04-28 11:54:40 -0400 (Wed, 28 Apr 2010) New Revision: 19315 Modified: search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/ContextHolder.java search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/EventListenerRegister.java search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java Log: HSEARCH-517 avoid using thread local when the event listeners are automatically registered Modified: search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/ContextHolder.java =================================================================== --- search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/ContextHolder.java 2010-04-28 14:33:12 UTC (rev 19314) +++ search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/ContextHolder.java 2010-04-28 15:54:40 UTC (rev 19315) @@ -24,16 +24,23 @@ */ package org.hibernate.search.event; +import java.util.Map; import java.util.WeakHashMap; import org.hibernate.cfg.Configuration; import org.hibernate.search.cfg.SearchConfigurationFromHibernateCore; +import org.hibernate.search.engine.SearchFactoryImplementor; import org.hibernate.search.impl.SearchFactoryImpl; /** * Holds already built SearchFactory per Hibernate Configuration object * concurrent threads do not share this information * + * This code uses ThreadLocal and despite the weak hashMap use, some users claim to see + * memory leaks (in Tomcat as usual). So if that can be avoided, do not use this class. + * + * There is no clean hook to always remove the SearchFactory from the map. + * * @author Emmanuel Bernard */ public class ContextHolder { @@ -55,4 +62,21 @@ } return searchFactory; } + + //code doesn't have to be multithreaded because SF creation is not. + //this is not a public API, should really only be used by the same + public static void removeSearchFactoryFromCache(SearchFactoryImplementor factory) { + WeakHashMap<Configuration, SearchFactoryImpl> contextMap = contexts.get(); + if ( contextMap != null ) { + for ( Map.Entry<Configuration, SearchFactoryImpl> entry : contextMap.entrySet() ) { + if ( entry.getValue() == factory ) { + contextMap.remove( entry.getKey() ); + } + } + //clear the thread local + if ( contextMap.size() == 0 ) { + contexts.remove(); + } + } + } } Modified: search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/EventListenerRegister.java =================================================================== --- search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/EventListenerRegister.java 2010-04-28 14:33:12 UTC (rev 19314) +++ search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/EventListenerRegister.java 2010-04-28 15:54:40 UTC (rev 19315) @@ -67,7 +67,9 @@ ); return; } - final FullTextIndexEventListener searchListener = new FullTextIndexEventListener(); + final FullTextIndexEventListener searchListener = + new FullTextIndexEventListener( FullTextIndexEventListener.Installation.SINGLE_INSTANCE ); + // PostInsertEventListener listeners.setPostInsertEventListeners( addIfNeeded( Modified: search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java =================================================================== --- search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java 2010-04-28 14:33:12 UTC (rev 19314) +++ search/trunk/hibernate-search/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java 2010-04-28 15:54:40 UTC (rev 19315) @@ -30,7 +30,6 @@ import java.io.Serializable; import java.lang.reflect.Field; import java.util.Map; - import javax.transaction.Status; import javax.transaction.Synchronization; @@ -61,10 +60,15 @@ import org.hibernate.search.backend.Work; import org.hibernate.search.backend.WorkType; import org.hibernate.search.backend.impl.EventSourceTransactionContext; +import org.hibernate.search.cfg.SearchConfigurationFromHibernateCore; import org.hibernate.search.engine.SearchFactoryImplementor; +import org.hibernate.search.impl.SearchFactoryImpl; import org.hibernate.search.util.LoggerFactory; import org.hibernate.search.util.WeakIdentityHashMap; +import static org.hibernate.search.event.FullTextIndexEventListener.Installation.MULTIPLE_INSTANCE; +import static org.hibernate.search.event.FullTextIndexEventListener.Installation.SINGLE_INSTANCE; + /** * This listener supports setting a parent directory for all generated index files. * It also supports setting the analyzer class to be used. @@ -86,6 +90,7 @@ protected boolean used; protected SearchFactoryImplementor searchFactoryImplementor; + private final Installation installation; //only used by the FullTextIndexEventListener instance playing in the FlushEventListener role. // transient because it's not serializable (and state doesn't need to live longer than a flush). @@ -94,12 +99,33 @@ // make sure the Synchronization doesn't contain references to Session, otherwise we'll leak memory. private transient final Map<Session,Synchronization> flushSynch = new WeakIdentityHashMap<Session,Synchronization>(0); + public FullTextIndexEventListener() { + this.installation = MULTIPLE_INSTANCE; + } + + public FullTextIndexEventListener(Installation installation) { + this.installation = installation; + } + /** * Initialize method called by Hibernate Core when the SessionFactory starts */ public void initialize(Configuration cfg) { - searchFactoryImplementor = ContextHolder.getOrBuildSearchFactory( cfg ); + /* + * if we know we pass the same instance, we can actually ensure that + * - initialize() is a no op if already run + * - avoid ContextHolder altogether + */ + if ( installation != SINGLE_INSTANCE ) { + log.debug( "Storing SearchFactory in ThreadLocal" ); + searchFactoryImplementor = ContextHolder.getOrBuildSearchFactory( cfg ); + } + else { + if ( searchFactoryImplementor == null ) { + searchFactoryImplementor = new SearchFactoryImpl( new SearchConfigurationFromHibernateCore( cfg ) ); + } + } String indexingStrategy = searchFactoryImplementor.getIndexingStrategy(); if ( "event".equals( indexingStrategy ) ) { used = searchFactoryImplementor.getDocumentBuildersIndexedEntities().size() != 0; @@ -107,6 +133,7 @@ else if ( "manual".equals( indexingStrategy ) ) { used = false; } + log.debug( "Hibernate Search event listeners " + (used ? "activated" : "desactivated") ); } public SearchFactoryImplementor getSearchFactoryImplementor() { @@ -153,8 +180,17 @@ public void cleanup() { searchFactoryImplementor.close(); + temptativeContextCleaning(); + } + private void temptativeContextCleaning() { + if ( installation != SINGLE_INSTANCE ) { + //unlikely to work: the thread used for closing is unlikely the thread used for initializing + ContextHolder.removeSearchFactoryFromCache( searchFactoryImplementor ); + } + } + public void onPostRecreateCollection(PostCollectionRecreateEvent event) { processCollectionEvent( event ); } @@ -242,6 +278,7 @@ private void writeObject(ObjectOutputStream os) throws IOException { os.defaultWriteObject(); + temptativeContextCleaning(); } //needs to implement custom readObject to restore the transient fields @@ -255,4 +292,8 @@ f.set( this, flushSynch ); } + public static enum Installation { + SINGLE_INSTANCE, + MULTIPLE_INSTANCE + } }