Author: psteitz Date: Sun Dec 21 20:24:54 2014 New Revision: 1647206 URL: http://svn.apache.org/r1647206 Log: Eliminated possibility that DefaultPoolObject#getIdleTimeMillis() could return a negative value. Use by pool implementations would not hit this bug. JIRA: POOL-279
Added: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java Modified: commons/proper/pool/trunk/src/changes/changes.xml commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java Modified: commons/proper/pool/trunk/src/changes/changes.xml URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/changes/changes.xml?rev=1647206&r1=1647205&r2=1647206&view=diff ============================================================================== --- commons/proper/pool/trunk/src/changes/changes.xml (original) +++ commons/proper/pool/trunk/src/changes/changes.xml Sun Dec 21 20:24:54 2014 @@ -43,7 +43,12 @@ The <action> type attribute can be add,u <title>Apache Commons Pool Changes</title> </properties> <body> - <release version="2.3" date="TBD" description="TBD"> + <release version="2.3" date="TBD" description="TBD"> + <action dev="psteitz" type="fix" issue="POOL-279" due-to="Jacopo Cappellato"> + Eliminated possibility that DefaultPoolObject#getIdleTimeMillis() could + return a negative value. Use by pool implementations would not hit this + bug. + </action> <action dev="psteitz" type="fix" issue="POOL-275"> Made wrapped BaseProxyHandler.pooledObject volatile. </action> Modified: commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java?rev=1647206&r1=1647205&r2=1647206&view=diff ============================================================================== --- commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java (original) +++ commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/DefaultPooledObject.java Sun Dec 21 20:24:54 2014 @@ -85,7 +85,11 @@ public class DefaultPooledObject<T> impl @Override public long getIdleTimeMillis() { - return System.currentTimeMillis() - lastReturnTime; + final long elapsed = System.currentTimeMillis() - lastReturnTime; + // elapsed may be negative if: + // - another thread updates lastReturnTime during the calculation window + // - System.currentTimeMillis() is not monotonic (e.g. system time is set back) + return elapsed >= 0 ? elapsed : 0; } @Override Added: commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java?rev=1647206&view=auto ============================================================================== --- commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java (added) +++ commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestDefaultPooledObject.java Sun Dec 21 20:24:54 2014 @@ -0,0 +1,84 @@ +/* + * 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.commons.pool2.impl; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.Assert; +import org.junit.Test; + +public class TestDefaultPooledObject { + + /** + * JIRA: POOL-279 + * @throws Exception + */ + @Test + public void testgetIdleTimeMillis() throws Exception { + final DefaultPooledObject<Object> dpo = new DefaultPooledObject<Object>(new Object()); + final AtomicBoolean negativeIdleTimeReturned = new AtomicBoolean(false); + ExecutorService executor = Executors.newFixedThreadPool( + Runtime.getRuntime().availableProcessors()*3); + Runnable allocateAndDeallocateTask = new Runnable() { + public void run() { + for (int i=0;i<10000;i++) { + if (dpo.getIdleTimeMillis() < 0) { + negativeIdleTimeReturned.set(true); + break; + } + } + dpo.allocate(); + for (int i=0;i<10000;i++) { + if (dpo.getIdleTimeMillis() < 0) { + negativeIdleTimeReturned.set(true); + break; + } + } + dpo.deallocate(); + } + }; + Runnable getIdleTimeTask = new Runnable() { + public void run() { + for (int i=0;i<10000;i++) { + if (dpo.getIdleTimeMillis() < 0) { + negativeIdleTimeReturned.set(true); + break; + } + } + } + }; + double probabilityOfAllocationTask = 0.7; + List<Future<?>> futures = new ArrayList<Future<?>>(); + for (int i = 1; i <= 10000; i++) { + Runnable randomTask = Math.random() < probabilityOfAllocationTask ? + allocateAndDeallocateTask : getIdleTimeTask; + futures.add(executor.submit(randomTask)); + } + for (Future<?> future: futures) { + future.get(); + } + Assert.assertFalse( + "DefaultPooledObject.getIdleTimeMillis() returned a negative value", + negativeIdleTimeReturned.get()); + } + +} \ No newline at end of file