SOLR-914: General finalize() improvements

git-svn-id: https://svn.apache.org/repos/asf/lucene/solr/trunk@807872 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -502,6 +502,11 @@ Bug Fixes
     effect QueryElevationComponentTest was refactored, and a bug in
     that test was found. (hossman)
 
+59. SOLR-914: General finalize() improvements.  No finalizer delegates
+    to the respective close/destroy method w/o first checking if it's
+    already been closed/destroyed; if it hasn't a, SEVERE error is
+    logged first.  (noble, hossman)
+
 Other Changes
 ----------------------
  1. Upgraded to Lucene 2.4.0 (yonik)

contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java

@@ -373,16 +373,23 @@ public class JdbcDataSource extends
 
   protected void finalize() throws Throwable {
     try {
-      conn.close();
+      if(!isClosed){
+        LOG.error("JdbcDataSource was not closed prior to finalize(), indicates a bug -- POSSIBLE RESOURCE LEAK!!!");
+        close();
+      }
     } finally {
       super.finalize();
     }
   }
 
+  private boolean isClosed = false;
   public void close() {
     try {
       conn.close();
     } catch (Exception e) {
+      LOG.error("Ignoring Error when closing connection", e);
+    } finally{
+      isClosed = true;
     }
 
   }

src/common/org/apache/solr/common/util/ConcurrentLRUCache.java

@@ -17,6 +17,8 @@ package org.apache.solr.common.util;
  */
 
 import org.apache.lucene.util.PriorityQueue;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -40,6 +42,7 @@ import java.lang.ref.WeakReference;
  * @since solr 1.4
  */
 public class ConcurrentLRUCache<K,V> {
+  private static Logger log = LoggerFactory.getLogger(ConcurrentLRUCache.class);
 
   private final ConcurrentHashMap<Object, CacheEntry> map;
   private final int upperWaterMark, lowerWaterMark;
@@ -490,10 +493,14 @@ public class ConcurrentLRUCache<K,V> {
     }
   }
 
-
+ private boolean isDestroyed =  false;
   public void destroy() {
-    if(cleanupThread != null){
-      cleanupThread.stopThread();
+    try {
+      if(cleanupThread != null){
+        cleanupThread.stopThread();
+      }
+    } finally {
+      isDestroyed = true;
     }
   }
 
@@ -583,8 +590,11 @@ public class ConcurrentLRUCache<K,V> {
 
   protected void finalize() throws Throwable {
     try {
-      destroy();
-    } finally { 
+      if(!isDestroyed){
+        log.error("ConcurrentLRUCache was not destroyed prior to finalize(), indicates a bug -- POSSIBLE RESOURCE LEAK!!!");
+        destroy();
+      }
+    } finally {
       super.finalize();
     }
   }

src/java/org/apache/solr/core/CoreContainer.java

@@ -271,24 +271,31 @@ public class CoreContainer
     }
     return properties;
   }
-
+  private boolean isShutDown = false;
   /**
    * Stops all cores.
    */
   public void shutdown() {
     synchronized(cores) {
-      for(SolrCore core : cores.values()) {
-        core.close();
+      try {
+        for(SolrCore core : cores.values()) {
+          core.close();
+        }
+        cores.clear();
+      } finally {
+        isShutDown = true;
       }
-      cores.clear();
     }
   }
   
   @Override
   protected void finalize() throws Throwable {
-    try { 
-      shutdown();
-    } finally { 
+    try {
+      if(!isShutDown){
+        log.error("CoreContainer was not shutdown prior to finalize(), indicates a bug -- POSSIBLE RESOURCE LEAK!!!");
+        shutdown();
+      }
+    } finally {
       super.finalize();
     }
   }

src/java/org/apache/solr/update/SolrIndexWriter.java

@@ -213,19 +213,26 @@ public class SolrIndexWriter extends IndexWriter {
    * }
    * ****
    */
-
+  private boolean isClosed = false;
   public void close() throws IOException {
     log.debug("Closing Writer " + name);
-    super.close();
-    if(infoStream != null) {
-      infoStream.close();
+    try {
+      super.close();
+      if(infoStream != null) {
+        infoStream.close();
+      }
+    } finally {
+      isClosed = true;
     }
   }
 
   @Override
   protected void finalize() throws Throwable {
     try {
-      super.close();
+      if(!isClosed){
+        log.error("SolrIndexWriter was not closed prior to finalize(), indicates a bug -- POSSIBLE RESOURCE LEAK!!!");
+        close();
+      }
     } finally { 
       super.finalize();
     }

src/test/org/apache/solr/search/TestFastLRUCache.java

@@ -51,6 +51,7 @@ public class TestFastLRUCache extends TestCase {
     scNew.init(l, o, cr);
     scNew.warm(null, sc);
     scNew.setState(SolrCache.State.LIVE);
+    sc.close();
     scNew.put(103, "103");
     assertEquals("90", scNew.get(90));
     assertEquals(null, scNew.get(50));
@@ -63,6 +64,7 @@ public class TestFastLRUCache extends TestCase {
     assertEquals(5L, nl.get("cumulative_lookups"));
     assertEquals(2L, nl.get("cumulative_hits"));
     assertEquals(102L, nl.get("cumulative_inserts"));
+    scNew.close();
   }
 
   public void testOldestItems() {
@@ -79,6 +81,7 @@ public class TestFastLRUCache extends TestCase {
     assertNotNull(m.get(5));
     assertNotNull(m.get(4));
     assertNotNull(m.get(2));
+    cache.destroy();
   }
 
   void doPerfTest(int iter, int cacheSize, int maxKey) {
@@ -102,6 +105,7 @@ public class TestFastLRUCache extends TestCase {
         else if (sz > maxSize) maxSize=sz;
       }
     }
+    cache.destroy();
 
     long end = System.currentTimeMillis();
     System.out.println("time=" + (end-start) + ", minSize="+minSize+",maxSize="+maxSize);