Author: markt Date: Mon Oct 14 21:42:33 2013 New Revision: 1532110 URL: http://svn.apache.org/r1532110 Log: Fix some FindBugs warnings
Modified: commons/proper/pool/trunk/findbugs-exclude-filter.xml commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/proxy/BaseProxyHandler.java commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftRefOutOfMemory.java commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftReferenceObjectPool.java Modified: commons/proper/pool/trunk/findbugs-exclude-filter.xml URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/findbugs-exclude-filter.xml?rev=1532110&r1=1532109&r2=1532110&view=diff ============================================================================== --- commons/proper/pool/trunk/findbugs-exclude-filter.xml (original) +++ commons/proper/pool/trunk/findbugs-exclude-filter.xml Mon Oct 14 21:42:33 2013 @@ -105,8 +105,41 @@ <Or> <Class name="org.apache.commons.pool2.impl.TestAbandonedObjectPool$ConcurrentBorrower" /> <Class name="org.apache.commons.pool2.impl.TestAbandonedObjectPool$ConcurrentReturner" /> + <Class name="org.apache.commons.pool2.impl.TestGenericKeyedObjectPool$SimpleTestThread" /> + <Class name="org.apache.commons.pool2.impl.TestGenericObjectPool$EvictionThread" /> </Or> <Method name="run" /> <Bug pattern="DE_MIGHT_IGNORE" /> </Match> + <Match> + <!-- Using equals is deliberate. The objects are being tested to see if --> + <!-- they are the same object --> + <Class name="org.apache.commons.pool2.impl.TestGenericKeyedObjectPool" /> + <Method name="testMaxTotalLRU" /> + <bug pattern="ES_COMPARING_STRINGS_WITH_EQ" /> + </Match> + <Match> + <!-- Exception is ignored as earlier exception is more important --> + <Class name="org.apache.commons.pool2.impl.TestGenericObjectPool" /> + <Method name="testMaxTotalUnderLoad" /> + <Bug pattern="DE_MIGHT_IGNORE" /> + </Match> + <Match> + <!-- Direct use of GC is deliberate and necessary --> + <Class name="org.apache.commons.pool2.impl.TestSoftRefOutOfMemory" /> + <Mehtod name="tearDown" /> + <Bug pattern="DM_GC" /> + </Match> + <Match> + <!-- Use of new String() is deliberate --> + <Class name="org.apache.commons.pool2.impl.TestSoftRefOutOfMemory$OomeFactory" /> + <Method name="create" /> + <Bug pattern="DM_STRING_VOID_CTOR" /> + </Match> + <Match> + <!-- Use of new String() is deliberate --> + <Class name="org.apache.commons.pool2.impl.TestSoftRefOutOfMemory$SmallPoolableObjectFactory" /> + <Method name="create" /> + <Bug pattern="DM_STRING_CTOR" /> + </Match> </FindBugsFilter> Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/proxy/BaseProxyHandler.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/proxy/BaseProxyHandler.java?rev=1532110&r1=1532109&r2=1532110&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/proxy/BaseProxyHandler.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/proxy/BaseProxyHandler.java Mon Oct 14 21:42:33 2013 @@ -50,7 +50,7 @@ class BaseProxyHandler<T> { void validateProxiedObject() { if (pooledObject == null) { throw new IllegalStateException("This object may no longer be " + - "used as it has been returned to the Object Pool."); + "used as it has been returned to the Object Pool."); } } Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java?rev=1532110&r1=1532109&r2=1532110&view=diff ============================================================================== --- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java (original) +++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java Mon Oct 14 21:42:33 2013 @@ -130,13 +130,7 @@ public class TestGenericObjectPool exten long timeBetweenEvictionRunsMillis = 8; boolean blockWhenExhausted = false; boolean lifo = false; - PooledObjectFactory<Object> factory = new BasePooledObjectFactory<Object>() { - @Override - public Object create() throws Exception { - return null; - } - }; - + PooledObjectFactory<Object> factory = new DummyFactory(); GenericObjectPool<Object> pool = new GenericObjectPool<Object>(factory); assertEquals(GenericObjectPoolConfig.DEFAULT_MAX_IDLE, pool.getMaxIdle()); @@ -1016,7 +1010,7 @@ public class TestGenericObjectPool exten Long[] creationTime = new Long[5] ; for(int i=0;i<5;i++) { active[i] = pool.borrowObject(); - creationTime[i] = new Long((active[i]).getCreateTime()); + creationTime[i] = Long.valueOf((active[i]).getCreateTime()); } for(int i=0;i<5;i++) { @@ -1037,23 +1031,6 @@ public class TestGenericObjectPool exten @Test(timeout=60000) public void testEvictionInvalid() throws Exception { - class InvalidFactory extends BasePooledObjectFactory<Object> { - - @Override - public Object create() throws Exception { - return new Object(); - } - - @Override - public boolean validateObject(PooledObject<Object> obj) { - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - // Ignore - } - return false; - } - } final GenericObjectPool<Object> pool = new GenericObjectPool<Object>(new InvalidFactory()); @@ -1070,16 +1047,7 @@ public class TestGenericObjectPool exten pool.returnObject(p); // Run eviction in a separate thread - Thread t = new Thread() { - @Override - public void run() { - try { - pool.evict(); - } catch (Exception e) { - // Ignore - } - } - }; + Thread t = new EvictionThread<Object>(pool); t.start(); // Sleep to make sure evictor has started @@ -1127,9 +1095,9 @@ public class TestGenericObjectPool exten final Random random = new Random(); for (int j = 0; j < nIterations; j++) { // Get a random invalidation target - Integer targ = new Integer(random.nextInt(nObjects)); + Integer targ = Integer.valueOf(random.nextInt(nObjects)); while (targets.contains(targ)) { - targ = new Integer(random.nextInt(nObjects)); + targ = Integer.valueOf(random.nextInt(nObjects)); } targets.add(targ); // Launch nThreads threads all trying to invalidate the target @@ -1953,4 +1921,51 @@ public class TestGenericObjectPool exten Set<ObjectName> result = mbs.queryNames(oname, null); Assert.assertEquals(1, result.size()); } + + + private static final class DummyFactory + extends BasePooledObjectFactory<Object> { + @Override + public Object create() throws Exception { + return null; + } + } + + + private static class InvalidFactory + extends BasePooledObjectFactory<Object> { + + @Override + public Object create() throws Exception { + return new Object(); + } + + @Override + public boolean validateObject(PooledObject<Object> obj) { + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + // Ignore + } + return false; + } + } + + private static class EvictionThread<T> extends Thread { + + private final GenericObjectPool<T> pool; + + public EvictionThread(GenericObjectPool<T> pool) { + this.pool = pool; + } + + @Override + public void run() { + try { + pool.evict(); + } catch (Exception e) { + // Ignore + } + } + } } Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftRefOutOfMemory.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftRefOutOfMemory.java?rev=1532110&r1=1532109&r2=1532110&view=diff ============================================================================== --- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftRefOutOfMemory.java (original) +++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftRefOutOfMemory.java Mon Oct 14 21:42:33 2013 @@ -5,9 +5,9 @@ * 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. @@ -162,12 +162,8 @@ public class TestSoftRefOutOfMemory { */ @Test public void testOutOfMemoryError() throws Exception { - pool = new SoftReferenceObjectPool<String>(new BasePooledObjectFactory<String>() { - @Override - public String create() throws Exception { - throw new OutOfMemoryError(); - } - }); + pool = new SoftReferenceObjectPool<String>( + new OomeFactory(OomeTrigger.CREATE)); try { pool.borrowObject(); @@ -178,17 +174,8 @@ public class TestSoftRefOutOfMemory { } pool.close(); - pool = new SoftReferenceObjectPool<String>(new BasePooledObjectFactory<String>() { - @Override - public String create() throws Exception { - return new String(); - } - - @Override - public boolean validateObject(PooledObject<String> obj) { - throw new OutOfMemoryError(); - } - }); + pool = new SoftReferenceObjectPool<String>( + new OomeFactory(OomeTrigger.VALIDATE)); try { pool.borrowObject(); @@ -198,23 +185,9 @@ public class TestSoftRefOutOfMemory { // expected } pool.close(); - - pool = new SoftReferenceObjectPool<String>(new BasePooledObjectFactory<String>() { - @Override - public String create() throws Exception { - return new String(); - } - @Override - public boolean validateObject(PooledObject<String> obj) { - throw new IllegalAccessError(); - } - - @Override - public void destroyObject(PooledObject<String> obj) throws Exception { - throw new OutOfMemoryError(); - } - }); + pool = new SoftReferenceObjectPool<String>( + new OomeFactory(OomeTrigger.DESTROY)); try { pool.borrowObject(); @@ -224,7 +197,6 @@ public class TestSoftRefOutOfMemory { // expected } pool.close(); - } @@ -259,4 +231,52 @@ public class TestSoftRefOutOfMemory { return String.valueOf(counter) + buffer; } } + + private static class OomeFactory extends BasePooledObjectFactory<String> { + + private final OomeTrigger trigger; + + public OomeFactory(OomeTrigger trigger) { + this.trigger = trigger; + } + + @Override + public String create() throws Exception { + if (trigger.equals(OomeTrigger.CREATE)) { + throw new OutOfMemoryError(); + } + // It seems that as of Java 1.4 String.valueOf may return an + // intern()'ed String this may cause problems when the tests + // depend on the returned object to be eventually garbaged + // collected. Either way, making sure a new String instance + // is returned eliminated false failures. + return new String(); + } + + @Override + public boolean validateObject(PooledObject<String> p) { + if (trigger.equals(OomeTrigger.VALIDATE)) { + throw new OutOfMemoryError(); + } + if (trigger.equals(OomeTrigger.DESTROY)) { + return false; + } else { + return true; + } + } + + @Override + public void destroyObject(PooledObject<String> p) throws Exception { + if (trigger.equals(OomeTrigger.DESTROY)) { + throw new OutOfMemoryError(); + } + super.destroyObject(p); + } + } + + private static enum OomeTrigger { + CREATE, + VALIDATE, + DESTROY + } } \ No newline at end of file Modified: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftReferenceObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftReferenceObjectPool.java?rev=1532110&r1=1532109&r2=1532110&view=diff ============================================================================== --- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftReferenceObjectPool.java (original) +++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestSoftReferenceObjectPool.java Mon Oct 14 21:42:33 2013 @@ -5,20 +5,19 @@ * 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.commons.pool2.impl; +import org.apache.commons.pool2.BasePooledObjectFactory; import org.apache.commons.pool2.ObjectPool; -import org.apache.commons.pool2.PooledObject; import org.apache.commons.pool2.PooledObjectFactory; import org.apache.commons.pool2.TestBaseObjectPool; @@ -29,21 +28,7 @@ public class TestSoftReferenceObjectPool @Override protected ObjectPool<Object> makeEmptyPool(int cap) { - return new SoftReferenceObjectPool<Object>( - new PooledObjectFactory<Object>() { - int counter = 0; - @Override - public PooledObject<Object> makeObject() { return new DefaultPooledObject<Object>(String.valueOf(counter++)); } - @Override - public void destroyObject(PooledObject<Object> obj) { } - @Override - public boolean validateObject(PooledObject<Object> obj) { return true; } - @Override - public void activateObject(PooledObject<Object> obj) { } - @Override - public void passivateObject(PooledObject<Object> obj) { } - } - ); + return new SoftReferenceObjectPool<Object>(new SimpleFactory()); } @Override @@ -66,4 +51,12 @@ public class TestSoftReferenceObjectPool return false; } + + private static class SimpleFactory extends BasePooledObjectFactory<Object> { + int counter = 0; + @Override + public Object create() { + return String.valueOf(counter++); + } + } }