Author: markt Date: Sun Nov 1 16:33:31 2009 New Revision: 831698 URL: http://svn.apache.org/viewvc?rev=831698&view=rev Log: Fix POOL-152. When borrowing an object if a new object is created but validate fails, the latch should not be returned to the queue as an exception will be thrown.
Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java?rev=831698&r1=831697&r2=831698&view=diff ============================================================================== --- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java (original) +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java Sun Nov 1 16:33:31 2009 @@ -1211,8 +1211,10 @@ } synchronized (this) { latch.getPool().decrementInternalProcessingCount(); - latch.reset(); - _allocationQueue.add(0, latch); + if (!newlyCreated) { + latch.reset(); + _allocationQueue.add(0, latch); + } allocate(); } if (newlyCreated) { Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java?rev=831698&r1=831697&r2=831698&view=diff ============================================================================== --- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java (original) +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java Sun Nov 1 16:33:31 2009 @@ -1181,8 +1181,10 @@ } synchronized (this) { _numInternalProcessing--; - latch.reset(); - _allocationQueue.add(0, latch); + if (!newlyCreated) { + latch.reset(); + _allocationQueue.add(0, latch); + } allocate(); } if(newlyCreated) { Modified: commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java?rev=831698&r1=831697&r2=831698&view=diff ============================================================================== --- commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java (original) +++ commons/proper/pool/trunk/src/test/org/apache/commons/pool/impl/TestGenericObjectPool.java Sun Nov 1 16:33:31 2009 @@ -1458,4 +1458,51 @@ } } } + + /** + * On first borrow, first object fails validation, second object is OK. + * Subsequent borrows are OK. This was POOL-152. + */ + public void testBrokenFactoryShouldNotBlockPool() { + int maxActive = 1; + + SimpleFactory factory = new SimpleFactory(); + factory.setMaxActive(maxActive); + pool.setFactory(factory); + pool.setMaxActive(maxActive); + pool.setWhenExhaustedAction(GenericObjectPool.WHEN_EXHAUSTED_BLOCK); + pool.setTestOnBorrow(true); + + // First borrow object will need to create a new object which will fail + // validation. + Object obj = null; + Exception ex = null; + factory.setValid(false); + try { + obj = pool.borrowObject(); + } catch (Exception e) { + ex = e; + } + // Failure expected + assertNotNull(ex); + assertTrue(ex instanceof NoSuchElementException); + assertNull(obj); + + // Configure factory to create valid objects so subsequent borrows work + factory.setValid(true); + + // Subsequent borrows should be OK + try { + obj = pool.borrowObject(); + } catch (Exception e1) { + fail(); + } + assertNotNull(obj); + try { + pool.returnObject(obj); + } catch (Exception e) { + fail(); + } + + } }