[hibernate-commits] Hibernate SVN: r16314 - search/trunk/src/main/java/org/hibernate/search/event.

[hibernate-commits at lists.jboss.org]

Author: sannegrinovero
Date: 2009-04-14 10:49:35 -0400 (Tue, 14 Apr 2009)
New Revision: 16314

Modified:
   search/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java
Log:
HSEARCH-178 changing from ThreadLocals to WeakIdentityHashMap

Modified: search/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java
===================================================================
--- search/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java	2009-04-14 13:51:42 UTC (rev 16313)
+++ search/trunk/src/main/java/org/hibernate/search/event/FullTextIndexEventListener.java	2009-04-14 14:49:35 UTC (rev 16314)
@@ -5,8 +5,8 @@
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
-import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
+import java.util.Map;
 
 import javax.transaction.Status;
 import javax.transaction.Synchronization;
@@ -40,6 +40,7 @@
 import org.hibernate.search.backend.impl.EventSourceTransactionContext;
 import org.hibernate.search.engine.SearchFactoryImplementor;
 import org.hibernate.search.util.LoggerFactory;
+import org.hibernate.search.util.WeakIdentityHashMap;
 
 /**
  * This listener supports setting a parent directory for all generated index files.
@@ -66,11 +67,9 @@
 	//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).
 	// final because it's initialization should be published to other threads.
-	// T.Local because different threads could be flushing on this listener, still the reference
-	// to session should be weak so that sessions which had errors on flush can be discarded.
 	// ! update the readObject() method in case of name changes !
-	// It's not static as we couldn't properly cleanup otherwise.
-	private transient final ThreadLocal<FlushContextContainer> flushSynch = new ThreadLocal<FlushContextContainer>();
+	// 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);
 
 	/**
 	 * Initialize method called by Hibernate Core when the SessionFactory starts
@@ -186,28 +185,19 @@
 	public void onFlush(FlushEvent event) {
 		if ( used ) {
 			Session session = event.getSession();
-			FlushContextContainer flushContextContainer = this.flushSynch.get();
-			if ( flushContextContainer != null ) {
-				//first cleanup the ThreadLocal
-				this.flushSynch.set( null );
-				EventSource registeringEventSource = flushContextContainer.eventSource.get();
-				//check that we are still in the same session which registered the flushSync:
-				if ( registeringEventSource != null && registeringEventSource == session ) {
-					log.debug( "flush event causing index update out of transaction" );
-					Synchronization synchronization = flushContextContainer.synchronization;
-					synchronization.beforeCompletion();
-					synchronization.afterCompletion( Status.STATUS_COMMITTED );
-				}
+			Synchronization synchronization = flushSynch.get( session );
+			if ( synchronization != null ) {
+				//first cleanup
+				flushSynch.remove( session );
+				log.debug( "flush event causing index update out of transaction" );
+				synchronization.beforeCompletion();
+				synchronization.afterCompletion( Status.STATUS_COMMITTED );
 			}
 		}
 	}
 
 	public void addSynchronization(EventSource eventSource, Synchronization synchronization) {
-		//no need to check for "unused" state, as this method is used by Search itself only.
-		FlushContextContainer flushContext = new FlushContextContainer(eventSource, synchronization);
-		//ignoring previously set data: if there was something, it's coming from a previous thread
-		//which had some error when flushing and couldn't cleanup.
-		this.flushSynch.set( flushContext );
+		this.flushSynch.put( eventSource, synchronization );
 	}
 
 	/* Might want to implement AutoFlushEventListener in future?
@@ -217,18 +207,6 @@
 	}
 	*/
 
-	private static class FlushContextContainer {
-		
-		private final WeakReference<EventSource> eventSource;
-		private final Synchronization synchronization;
-
-		public FlushContextContainer(EventSource eventSource, Synchronization synchronization) {
-			this.eventSource = new WeakReference<EventSource>( eventSource );
-			this.synchronization = synchronization;
-		}
-		
-	}
-
 	private void writeObject(ObjectOutputStream os) throws IOException {
 		os.defaultWriteObject();
 	}
@@ -239,7 +217,7 @@
 		Class<FullTextIndexEventListener> cl = FullTextIndexEventListener.class;
 		Field f = cl.getDeclaredField("flushSynch");
 		f.setAccessible( true );
-		ThreadLocal<FlushContextContainer> flushSynch = new ThreadLocal<FlushContextContainer>();
+		Map<Session,Synchronization> flushSynch = new WeakIdentityHashMap<Session,Synchronization>(0);
 		// setting a final field by reflection during a readObject is considered as safe as in a constructor:
 		f.set( this, flushSynch );
 	}