diff --git a/CHANGES.txt b/CHANGES.txt index e3bae4aa3c2..0a11362a7e7 100644 --- a/CHANGES.txt +++ b/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) diff --git a/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java b/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java index 8347df18409..b394f70451d 100644 --- a/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/JdbcDataSource.java +++ b/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; } } diff --git a/src/common/org/apache/solr/common/util/ConcurrentLRUCache.java b/src/common/org/apache/solr/common/util/ConcurrentLRUCache.java index 1c30886b198..1ca39693904 100644 --- a/src/common/org/apache/solr/common/util/ConcurrentLRUCache.java +++ b/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 { + private static Logger log = LoggerFactory.getLogger(ConcurrentLRUCache.class); private final ConcurrentHashMap map; private final int upperWaterMark, lowerWaterMark; @@ -490,10 +493,14 @@ public class ConcurrentLRUCache { } } - + 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 { 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(); } } diff --git a/src/java/org/apache/solr/core/CoreContainer.java b/src/java/org/apache/solr/core/CoreContainer.java index 8cdfa35be94..53111d98605 100644 --- a/src/java/org/apache/solr/core/CoreContainer.java +++ b/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(); } } diff --git a/src/java/org/apache/solr/update/SolrIndexWriter.java b/src/java/org/apache/solr/update/SolrIndexWriter.java index 8a0603339e7..56e179ba294 100644 --- a/src/java/org/apache/solr/update/SolrIndexWriter.java +++ b/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(); } diff --git a/src/test/org/apache/solr/search/TestFastLRUCache.java b/src/test/org/apache/solr/search/TestFastLRUCache.java index 7222d83a51e..99841a34a00 100644 --- a/src/test/org/apache/solr/search/TestFastLRUCache.java +++ b/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);