[jira] [Comment Edited] (POOL-340) borrowObject is stuck, if create fails

["Pavel Kolesov (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/POOL-340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16458398#comment-16458398 ] 

Pavel Kolesov edited comment on POOL-340 at 4/30/18 8:32 AM:
-------------------------------------------------------------

[~garydgregory], thank you for your response and sorry for the delay. I've come up with such test (not the shortest one, unfortunately):
{code}

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.commons.pool2.BasePooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;

public class GenericObjectPoolTest {
    
    @Test(timeout = 10_000)
    @Ignore("Doesn't pass, waiting for https://issues.apache.org/jira/browse/POOL-340")
    public void testBorrowObjectStuck() {
        SingleObjectFactory factory = new SingleObjectFactory();
        GenericObjectPoolConfig config = new GenericObjectPoolConfig();
        config.setMaxIdle(1);
        config.setMaxTotal(1);
        config.setBlockWhenExhausted(true);
        config.setMinIdle(0);
        config.setTestOnBorrow(true);
        config.setTestOnReturn(true);
        config.setTestWhileIdle(false);
        config.setTimeBetweenEvictionRunsMillis(-1);
        config.setMinEvictableIdleTimeMillis(-1);
        config.setSoftMinEvictableIdleTimeMillis(-1);
        
        config.setMaxWaitMillis(-1);
        GenericObjectPool<Object> pool = new GenericObjectPool<>(factory, config);
        
        AtomicBoolean failed = new AtomicBoolean();
        CountDownLatch barrier = new CountDownLatch(1);
        Thread thread1 = new Thread(new WinnerRunnable(pool, barrier, failed));
        thread1.start();
        
        // wait for object to be created
        while(!factory.created.get()) {
            sleepIgnoreException(5);
        }
        
        // now borrow
        barrier.countDown();
        try {
            pool.borrowObject();
        } catch (Exception e) {
            e.printStackTrace();
        }
        
        Assert.assertFalse(failed.get());
        
    }
    
    private static class SingleObjectFactory extends BasePooledObjectFactory<Object> {
        private final AtomicBoolean created = new AtomicBoolean();
        private final AtomicBoolean validated = new AtomicBoolean();
        @Override
        public Object create() throws Exception {
            if (!created.getAndSet(true)) {
                return new Object();
            }
            throw new Exception("Already created");
        }

        @Override
        public PooledObject<Object> wrap(Object obj) {
            return new DefaultPooledObject<>(new Object());
        }

        @Override
        public boolean validateObject(PooledObject<Object> p) {
            // return valid once
            if (!validated.getAndSet(true)) {
                return true;
            }
            System.out.println("invalid");
            return false;
        }
    }
    
    private static class WinnerRunnable implements Runnable {
        private final GenericObjectPool<Object> pool;
        private final AtomicBoolean failed;
        private final CountDownLatch barrier;
        private WinnerRunnable(GenericObjectPool<Object> pool, CountDownLatch barrier, AtomicBoolean failed) {
            this.pool = pool;
            this.failed = failed;
            this.barrier = barrier;
        }
        @Override
        public void run() {
            try {
                Object obj = pool.borrowObject();
                
                // wait for another thread to start borrowObject
                if (!barrier.await(5, TimeUnit.SECONDS)) {
                    System.out.println("Timeout waiting");
                    failed.set(true);
                } else {
                    // just to make sure, borrowObject has started waiting on queue
                    sleepIgnoreException(1000);
                }
                
                pool.returnObject(obj);
            } catch (Exception e) {
                failed.set(true);
                e.printStackTrace();
            }
        }
    }
    
    private static void sleepIgnoreException(long millis) {
        try {
            Thread.sleep(millis);
        } catch(Throwable e) {
            // ignore
        }
    }
}
{code}


was (Author: pavelk):
[~garydgregory], thank you for your response and sorry for the delay. I've come up with such test (not the shortest one, unfortunately):
{code}
package org.apache.commons.pool2.impl;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.commons.pool2.BasePooledObjectFactory;
import org.apache.commons.pool2.PooledObject;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;

public class GenericObjectPoolTest {
    
    @Test(timeout = 10_000)
    @Ignore("Doesn't pass, waiting for https://issues.apache.org/jira/browse/POOL-340")
    public void testBorrowObjectStuck() {
        SingleObjectFactory factory = new SingleObjectFactory();
        GenericObjectPoolConfig config = new GenericObjectPoolConfig();
        config.setMaxIdle(1);
        config.setMaxTotal(1);
        config.setBlockWhenExhausted(true);
        config.setMinIdle(0);
        config.setTestOnBorrow(true);
        config.setTestOnReturn(true);
        config.setTestWhileIdle(false);
        config.setTimeBetweenEvictionRunsMillis(-1);
        config.setMinEvictableIdleTimeMillis(-1);
        config.setSoftMinEvictableIdleTimeMillis(-1);
        
        config.setMaxWaitMillis(-1);
        GenericObjectPool<Object> pool = new GenericObjectPool<>(factory, config);
        
        AtomicBoolean failed = new AtomicBoolean();
        CountDownLatch barrier = new CountDownLatch(1);
        Thread thread1 = new Thread(new WinnerRunnable(pool, barrier, failed));
        thread1.start();
        
        // wait for object to be created
        while(!factory.created.get()) {
            sleepIgnoreException(5);
        }
        
        // now borrow
        barrier.countDown();
        try {
            pool.borrowObject();
        } catch (Exception e) {
            e.printStackTrace();
        }
        
        Assert.assertFalse(failed.get());
        
    }
    
    private static class SingleObjectFactory extends BasePooledObjectFactory<Object> {
        private final AtomicBoolean created = new AtomicBoolean();
        private final AtomicBoolean validated = new AtomicBoolean();
        @Override
        public Object create() throws Exception {
            if (!created.getAndSet(true)) {
                return new Object();
            }
            throw new Exception("Already created");
        }

        @Override
        public PooledObject<Object> wrap(Object obj) {
            return new DefaultPooledObject<>(new Object());
        }

        @Override
        public boolean validateObject(PooledObject<Object> p) {
            // return valid once
            if (!validated.getAndSet(true)) {
                return true;
            }
            System.out.println("invalid");
            return false;
        }
    }
    
    private static class WinnerRunnable implements Runnable {
        private final GenericObjectPool<Object> pool;
        private final AtomicBoolean failed;
        private final CountDownLatch barrier;
        private WinnerRunnable(GenericObjectPool<Object> pool, CountDownLatch barrier, AtomicBoolean failed) {
            this.pool = pool;
            this.failed = failed;
            this.barrier = barrier;
        }
        @Override
        public void run() {
            try {
                Object obj = pool.borrowObject();
                
                // wait for another thread to start borrowObject
                if (!barrier.await(5, TimeUnit.SECONDS)) {
                    System.out.println("Timeout waiting");
                    failed.set(true);
                } else {
                    // just to make sure, borrowObject has started waiting on queue
                    sleepIgnoreException(1000);
                }
                
                pool.returnObject(obj);
            } catch (Exception e) {
                failed.set(true);
                e.printStackTrace();
            }
        }
    }
    
    private static void sleepIgnoreException(long millis) {
        try {
            Thread.sleep(millis);
        } catch(Throwable e) {
            // ignore
        }
    }
}
{code}

> borrowObject is stuck, if create fails
> --------------------------------------
>
>                 Key: POOL-340
>                 URL: https://issues.apache.org/jira/browse/POOL-340
>             Project: Commons Pool
>          Issue Type: Bug
>    Affects Versions: 2.4, 2.4.1, 2.4.2, 2.4.3, 2.5.0
>            Reporter: Pavel Kolesov
>            Priority: Critical
>
> After changes in 2.4.3 there is a high chance of a scenario, in which borrowObject waits infinitely, if create fails or no one calls a create.
> {noformat}
>    java.lang.Thread.State: WAITING (parking)
>         at sun.misc.Unsafe.park(Native Method)
>         - parking to wait for  <0x0000000083cfd978> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
>         at java.util.concurrent.locks.LockSupport.park(LockSupport.java:175)
>         at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.await(AbstractQueuedSynchronizer.java:2039)
>         at org.apache.commons.pool2.impl.LinkedBlockingDeque.takeFirst(LinkedBlockingDeque.java:583)
>         at org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:442)
>         at org.apache.commons.pool2.impl.GenericObjectPool.borrowObject(GenericObjectPool.java:363)
> {noformat}
> If pool is exhausted, when borrowObject tries to get idle object, it waits for new object to be created.
> If all objects are returned to pool invalid and destroyed, and it is impossible to create a new one, borrowObject will not return.
> Even if afterwards it is becomes possible to crate a new object but no one creates it, borrowObject will not return either.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)