From 3f79aec1961d0167eabde838fed96f64900e9f99 Mon Sep 17 00:00:00 2001 From: Yonik Seeley Date: Tue, 19 Oct 2010 17:42:27 +0000 Subject: [PATCH] tests: src/test/org/apache/solr/util/SolrPluginUtilsTest.javafix resource leak git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1024335 13f79535-47bb-0310-9956-ffa450edef68 --- .../solr/common/util/ConcurrentLRUCache.java | 7 +- .../apache/solr/core/QuerySenderListener.java | 2 +- .../solr/request/SolrQueryRequestBase.java | 16 +++ .../apache/solr/search/SolrIndexSearcher.java | 5 + .../org/apache/solr/SolrInfoMBeanTest.java | 115 ------------------ .../test/org/apache/solr/SolrTestCaseJ4.java | 24 +++- .../apache/solr/util/SolrPluginUtilsTest.java | 5 +- solr/testlogging.properties | 2 +- 8 files changed, 54 insertions(+), 122 deletions(-) delete mode 100644 solr/src/test/org/apache/solr/SolrInfoMBeanTest.java diff --git a/solr/src/common/org/apache/solr/common/util/ConcurrentLRUCache.java b/solr/src/common/org/apache/solr/common/util/ConcurrentLRUCache.java index ed352f2e186..6bc83dea149 100644 --- a/solr/src/common/org/apache/solr/common/util/ConcurrentLRUCache.java +++ b/solr/src/common/org/apache/solr/common/util/ConcurrentLRUCache.java @@ -59,6 +59,8 @@ public class ConcurrentLRUCache { public ConcurrentLRUCache(int upperWaterMark, final int lowerWaterMark, int acceptableWatermark, int initialSize, boolean runCleanupThread, boolean runNewThreadForCleanup, EvictionListener evictionListener) { +log.info("new ConcurrentLRUCache: " + this); + if (upperWaterMark < 1) throw new IllegalArgumentException("upperWaterMark must be > 0"); if (lowerWaterMark >= upperWaterMark) throw new IllegalArgumentException("lowerWaterMark must be < upperWaterMark"); @@ -500,8 +502,9 @@ public class ConcurrentLRUCache { } } - private boolean isDestroyed = false; + private volatile boolean isDestroyed = false; public void destroy() { + log.info("destroying " + this); try { if(cleanupThread != null){ cleanupThread.stopThread(); @@ -607,7 +610,7 @@ public class ConcurrentLRUCache { protected void finalize() throws Throwable { try { if(!isDestroyed){ - log.error("ConcurrentLRUCache was not destroyed prior to finalize(), indicates a bug -- POSSIBLE RESOURCE LEAK!!!"); + log.error("ConcurrentLRUCache was not destroyed prior to finalize(), indicates a bug -- POSSIBLE RESOURCE LEAK!!! - " + this); destroy(); } } finally { diff --git a/solr/src/java/org/apache/solr/core/QuerySenderListener.java b/solr/src/java/org/apache/solr/core/QuerySenderListener.java index 69a40ba2efa..ff7f3fe0baf 100644 --- a/solr/src/java/org/apache/solr/core/QuerySenderListener.java +++ b/solr/src/java/org/apache/solr/core/QuerySenderListener.java @@ -45,7 +45,7 @@ class QuerySenderListener extends AbstractSolrEventListener { NamedList params = addEventParms(currentSearcher, nlst); LocalSolrQueryRequest req = new LocalSolrQueryRequest(core,params) { @Override public SolrIndexSearcher getSearcher() { return searcher; } - @Override public void close() { } + // @Override public void close() { } }; SolrQueryResponse rsp = new SolrQueryResponse(); diff --git a/solr/src/java/org/apache/solr/request/SolrQueryRequestBase.java b/solr/src/java/org/apache/solr/request/SolrQueryRequestBase.java index 4dcaafea614..a89298bf80d 100644 --- a/solr/src/java/org/apache/solr/request/SolrQueryRequestBase.java +++ b/solr/src/java/org/apache/solr/request/SolrQueryRequestBase.java @@ -232,8 +232,22 @@ public abstract class SolrQueryRequestBase implements SolrQueryRequest { searcherHolder.decref(); searcherHolder = null; } + allocator = null; } + public volatile Exception allocator; + { + allocator = new RuntimeException("WhoAmI"); + allocator.fillInStackTrace(); + } + @Override + protected void finalize() throws Throwable { + if (allocator != null) { + SolrException.log(SolrCore.log, "MISSING CLOSE for req allocated at ", allocator); + } + } + + /** A Collection of ContentStreams passed to the request */ public Iterable getContentStreams() { @@ -252,4 +266,6 @@ public abstract class SolrQueryRequestBase implements SolrQueryRequest { return this.getClass().getSimpleName() + '{' + params + '}'; } + + } diff --git a/solr/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/src/java/org/apache/solr/search/SolrIndexSearcher.java index 869046ef7b8..b5ca87b4a06 100644 --- a/solr/src/java/org/apache/solr/search/SolrIndexSearcher.java +++ b/solr/src/java/org/apache/solr/search/SolrIndexSearcher.java @@ -37,6 +37,7 @@ import org.apache.lucene.util.OpenBitSet; import java.io.IOException; import java.net.URL; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import org.apache.solr.search.function.ValueSource; @@ -57,6 +58,8 @@ public class SolrIndexSearcher extends IndexSearcher implements SolrInfoMBean { public static final AtomicLong numOpens = new AtomicLong(); public static final AtomicLong numCloses = new AtomicLong(); + public static Map openSearchers = new ConcurrentHashMap(); + private static Logger log = LoggerFactory.getLogger(SolrIndexSearcher.class); private final SolrCore core; @@ -138,6 +141,7 @@ public class SolrIndexSearcher extends IndexSearcher implements SolrInfoMBean { public SolrIndexSearcher(SolrCore core, IndexSchema schema, String name, IndexReader r, boolean closeReader, boolean enableCache) { super(wrap(r)); +openSearchers.put(this, new RuntimeException("SearcherAlloc").fillInStackTrace()); this.reader = (SolrIndexReader)super.getIndexReader(); this.core = core; this.schema = schema; @@ -228,6 +232,7 @@ public class SolrIndexSearcher extends IndexSearcher implements SolrInfoMBean { * In particular, the underlying reader and any cache's in use are closed. */ public void close() throws IOException { + openSearchers.remove(this); if (cachingEnabled) { StringBuilder sb = new StringBuilder(); sb.append("Closing ").append(name); diff --git a/solr/src/test/org/apache/solr/SolrInfoMBeanTest.java b/solr/src/test/org/apache/solr/SolrInfoMBeanTest.java deleted file mode 100644 index 2177f823a7f..00000000000 --- a/solr/src/test/org/apache/solr/SolrInfoMBeanTest.java +++ /dev/null @@ -1,115 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.solr; - -import org.apache.lucene.util.LuceneTestCase; -import org.apache.solr.core.SolrInfoMBean; -import org.apache.solr.handler.StandardRequestHandler; -import org.apache.solr.handler.admin.LukeRequestHandler; -import org.apache.solr.handler.component.SearchComponent; -import org.apache.solr.handler.component.SearchHandler; -import org.apache.solr.highlight.DefaultSolrHighlighter; -import org.apache.solr.search.LRUCache; -import org.junit.Ignore; - -import java.io.File; -import java.net.URL; -import java.util.ArrayList; -import java.util.Enumeration; -import java.util.List; - -/** - * A simple test used to increase code coverage for some standard things... - */ -public class SolrInfoMBeanTest extends LuceneTestCase -{ - /** - * Gets a list of everything we can find in the classpath and makes sure it has - * a name, description, etc... - */ - public void testCallMBeanInfo() throws Exception { - List classes = new ArrayList(); - classes.addAll(getClassesForPackage(StandardRequestHandler.class.getPackage().getName())); - classes.addAll(getClassesForPackage(SearchHandler.class.getPackage().getName())); - classes.addAll(getClassesForPackage(SearchComponent.class.getPackage().getName())); - classes.addAll(getClassesForPackage(LukeRequestHandler.class.getPackage().getName())); - classes.addAll(getClassesForPackage(DefaultSolrHighlighter.class.getPackage().getName())); - classes.addAll(getClassesForPackage(LRUCache.class.getPackage().getName())); - // System.out.println(classes); - - int checked = 0; - for( Class clazz : classes ) { - if( SolrInfoMBean.class.isAssignableFrom( clazz ) ) { - try { - SolrInfoMBean info = (SolrInfoMBean)clazz.newInstance(); - - //System.out.println( info.getClass() ); - assertNotNull( info.getName() ); - assertNotNull( info.getDescription() ); - assertNotNull( info.getSource() ); - assertNotNull( info.getSourceId() ); - assertNotNull( info.getVersion() ); - assertNotNull( info.getCategory() ); - - if( info instanceof LRUCache ) { - continue; - } - - assertNotNull( info.toString() ); - // increase code coverage... - assertNotNull( info.getDocs() + "" ); - assertNotNull( info.getStatistics()+"" ); - checked++; - } - catch( InstantiationException ex ) { - // expected... - //System.out.println( "unable to initalize: "+clazz ); - } - } - } - assertTrue( "there are at least 10 SolrInfoMBean that should be found in the classpath, found " + checked, checked > 10 ); - } - - static final String FOLDER = File.separator + "build" + File.separator + "solr" + File.separator + "org" + File.separator + "apache" + File.separator + "solr" + File.separator; - - private static List getClassesForPackage(String pckgname) throws Exception { - ArrayList directories = new ArrayList(); - ClassLoader cld = Thread.currentThread().getContextClassLoader(); - String path = pckgname.replace('.', '/'); - Enumeration resources = cld.getResources(path); - while (resources.hasMoreElements()) { - final File f = new File(resources.nextElement().toURI()); - // only iterate classes from the core, not the tests (must be in dir "/build/solr/org" - if (!f.toString().contains(FOLDER)) - continue; - directories.add(f); - } - - ArrayList classes = new ArrayList(); - for (File directory : directories) { - if (directory.exists()) { - String[] files = directory.list(); - for (String file : files) { - if (file.endsWith(".class")) { - classes.add(Class.forName(pckgname + '.' + file.substring(0, file.length() - 6))); - } - } - } - } - return classes; - } -} diff --git a/solr/src/test/org/apache/solr/SolrTestCaseJ4.java b/solr/src/test/org/apache/solr/SolrTestCaseJ4.java index f5ff9c85bc8..5e953547d18 100755 --- a/solr/src/test/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/src/test/org/apache/solr/SolrTestCaseJ4.java @@ -30,6 +30,7 @@ import org.apache.solr.core.SolrConfig; import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.util.RefCounted; import org.apache.solr.util.TestHarness; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -44,6 +45,7 @@ import java.io.StringWriter; import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -64,6 +66,10 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { public static void afterClassSolrTestCase() throws Exception { deleteCore(); resetExceptionIgnores(); + for (Map.Entry entry : SolrIndexSearcher.openSearchers.entrySet()) { + log.error("ERROR SEARCHER="+entry.getKey()); + SolrException.log(log, "SEARCHER ALLOCED AT ", entry.getValue()); + } } @Override @@ -238,6 +244,10 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { ("standard",0,20,"version","2.2"); } log.info("####initCore end"); + + RefCounted holder = h.getCore().getSearcher(); + log.info("START SEARCHER REFCOUNT=" + (holder.getRefcount()-1) + " instance="+holder.get()); + holder.decref(); } /** Subclasses that override setUp can optionally call this method @@ -264,7 +274,13 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { */ public static void deleteCore() throws Exception { log.info("###deleteCore" ); - if (h != null) { h.close(); } + RefCounted holder = null; + + if (h != null) { + holder = h.getCore().getSearcher(); + log.info("END SEARCHER REFCOUNT=" + (holder.getRefcount()-1) + " instance="+holder.get()); + h.close(); + } if (dataDir != null) { String skip = System.getProperty("solr.test.leavedatadir"); if (null != skip && 0 != skip.trim().length()) { @@ -286,6 +302,12 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { lrf = null; configString = schemaString = null; + + if (holder != null) { + log.info("FINAL SEARCHER REFCOUNT=" + (holder.getRefcount()-1) + " instance="+holder.get()); + holder.decref(); + } + endTrackingSearchers(); } diff --git a/solr/src/test/org/apache/solr/util/SolrPluginUtilsTest.java b/solr/src/test/org/apache/solr/util/SolrPluginUtilsTest.java index 0fd5e9422c7..325f1f6354b 100644 --- a/solr/src/test/org/apache/solr/util/SolrPluginUtilsTest.java +++ b/solr/src/test/org/apache/solr/util/SolrPluginUtilsTest.java @@ -66,7 +66,8 @@ public class SolrPluginUtilsTest extends SolrTestCaseJ4 { assertU("", adoc("id", "3235", "val_t", "quick green fox")); assertU("", adoc("id", "3236", "val_t", "quick brown fox")); commit(); - SolrIndexSearcher srchr = h.getCore().getSearcher().get(); + RefCounted holder = h.getCore().getSearcher(); + SolrIndexSearcher srchr = holder.get(); SolrIndexSearcher.QueryResult qr = new SolrIndexSearcher.QueryResult(); SolrIndexSearcher.QueryCommand cmd = new SolrIndexSearcher.QueryCommand(); cmd.setQuery(new MatchAllDocsQuery()); @@ -82,7 +83,7 @@ public class SolrPluginUtilsTest extends SolrTestCaseJ4 { for (SolrDocument document : list) { assertNotNull(document.get("val_t")); } - srchr.close(); + holder.close(); } @Test diff --git a/solr/testlogging.properties b/solr/testlogging.properties index 4a09a265675..44405e59f70 100644 --- a/solr/testlogging.properties +++ b/solr/testlogging.properties @@ -1,4 +1,4 @@ handlers=java.util.logging.ConsoleHandler -.level=SEVERE +.level=INFO java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter