commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Mueller (JIRA)" <j...@apache.org>
Subject [jira] Updated: (POOL-132) GenericKeyedObjectPool.borrowObject(Object key) blocked for all threads until _factory.makeObject(key) is finished
Date Thu, 15 Jan 2009 13:26:59 GMT

     [ https://issues.apache.org/jira/browse/POOL-132?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Michael Mueller updated POOL-132:
---------------------------------

    Description: 
If no Object is in the pool the method GenericKeyedObjectPool.borrowObject(Object key) is
blocking until a new Object (_factory.makeObject(key) ) is created.  This will block all other
threads even if they want an Object for a different key or want return an Object.

The GenericObjectPool.borrowObject() have a better implementation where the _factory.makeObject(key)
 is called after the synchronized block.

Could this changed like this?

{code:title=GenericKeyedObjectPool.borrowObject(Object key)|borderStyle=solid}

    public Object borrowObject(Object key) throws Exception {
        long starttime = System.currentTimeMillis();
        
        for(;;) {
            ObjectTimestampPair pair = null;
            ObjectQueue pool = null;
            synchronized (this) {
                assertOpen();
                pool = (ObjectQueue)(_poolMap.get(key));
                if(null == pool) {
                    pool = new ObjectQueue();
                    _poolMap.put(key,pool);
                    _poolList.add(key);
                }
                // if there are any sleeping, just grab one of those
                try {
                    pair = (ObjectTimestampPair)(pool.queue.removeFirst());
                    if(null != pair) {
                        _totalIdle--;
                    }
                } catch(NoSuchElementException e) { /* ignored */
                }
                // otherwise
                if(null == pair) {
                    // if there is a totalMaxActive and we are at the limit then
                    // we have to make room
                    if ((_maxTotal > 0) && (_totalActive + _totalIdle >= _maxTotal))
{
                        clearOldest();
                    }
    
                    // check if we can create one
                    // (note we know that the num sleeping is 0, else we wouldn't be here)
                    if ((_maxActive < 0 || pool.activeCount < _maxActive) &&
                        (_maxTotal < 0 || _totalActive + _totalIdle < _maxTotal)) {
                        // allow new object to be created
                    } else {
                        // the pool is exhausted
                        switch(_whenExhaustedAction) {
                            case WHEN_EXHAUSTED_GROW:
                                // allow new object to be created
                                break;
                            case WHEN_EXHAUSTED_FAIL:
                                throw new NoSuchElementException();
                            case WHEN_EXHAUSTED_BLOCK:
                                try {
                                    if(_maxWait <= 0) {
                                        wait();
                                    } else {
                                        // this code may be executed again after a notify
then continue cycle
                                        // so, need to calculate the amount of time to wait
                                        final long elapsed = (System.currentTimeMillis() -
starttime);
                                        final long waitTime = _maxWait - elapsed;
                                        if (waitTime > 0)
                                        {
                                            wait(waitTime);
                                        }
                                    }
                                } catch(InterruptedException e) {
                                    // ignored
                                }
                                if(_maxWait > 0 && ((System.currentTimeMillis()
- starttime) >= _maxWait)) {
                                    throw new NoSuchElementException("Timeout waiting for
idle object");
                                } else {
                                    continue; // keep looping
                                }
                            default:
                                throw new IllegalArgumentException("whenExhaustedAction "
+ _whenExhaustedAction + " not recognized.");
                        }
                    }
                }
                pool.incrementActiveCount();
            }
            
            // create new object when needed
            boolean newlyCreated = false;
            if(null == pair) {
                try {
                    Object obj = _factory.makeObject();
                    pair = new ObjectTimestampPair(obj);
                    newlyCreated = true;
                } finally {
                    if (!newlyCreated) {
                        // object cannot be created
                        synchronized (this) {
                            pool.decrementActiveCount();                            
                        }
                    }
                }
            }

            
            // Activate.  If activate fails, decrement active count and destroy.
            // If instance failing activation is new, throw NoSuchElementException;
            // otherwise keep looping
            try {
                _factory.activateObject(key, pair.value);
            } catch (Exception e) {
                try {
                    _factory.destroyObject(key,pair.value);
                    synchronized (this) {
                        pool.decrementActiveCount();
                    }
                } catch (Exception e2) {
                    // swallowed
                }
                if(newlyCreated) {
                    throw new NoSuchElementException(
                       "Could not create a validated object, cause: "
                            + e.getMessage());
                }
                else {
                    continue; // keep looping
                }
            }

            // Validate.  If validation fails, decrement active count and
            // destroy. If instance failing validation is new, throw
            // NoSuchElementException; otherwise keep looping
            boolean invalid = true;
            try {
                invalid = _testOnBorrow && !_factory.validateObject(key, pair.value);
            } catch (Exception e) {
                // swallowed
            }
            if (invalid) {
                try {
                    _factory.destroyObject(key,pair.value);
                    synchronized (this) {
                        pool.decrementActiveCount();
                    }
                } catch (Exception e) {
                    // swallowed
                }
                if(newlyCreated) {
                    throw new NoSuchElementException("Could not create a validated object");
                } // else keep looping
            } else {
                return pair.value;
            }
        }
    }

{code} 



  was:
If no Object is in the pool the method GenericKeyedObjectPool.borrowObject(Object key) is
blocking until a new Object (_factory.makeObject(key) ) is created.  This will block all other
threads even if they want an Object for a different key or want return an Object.

The GenericObjectPool.borrowObject() have a better implementation where the _factory.makeObject(key)
 is called after the synchronized block.

Could this changed like this?
<pre>
    public Object borrowObject(Object key) throws Exception {
        long starttime = System.currentTimeMillis();
        
        for(;;) {
            ObjectTimestampPair pair = null;
            ObjectQueue pool = null;
            synchronized (this) {
                assertOpen();
                pool = (ObjectQueue)(_poolMap.get(key));
                if(null == pool) {
                    pool = new ObjectQueue();
                    _poolMap.put(key,pool);
                    _poolList.add(key);
                }
                // if there are any sleeping, just grab one of those
                try {
                    pair = (ObjectTimestampPair)(pool.queue.removeFirst());
                    if(null != pair) {
                        _totalIdle--;
                    }
                } catch(NoSuchElementException e) { /* ignored */
                }
                // otherwise
                if(null == pair) {
                    // if there is a totalMaxActive and we are at the limit then
                    // we have to make room
                    if ((_maxTotal > 0) && (_totalActive + _totalIdle >= _maxTotal))
{
                        clearOldest();
                    }
    
                    // check if we can create one
                    // (note we know that the num sleeping is 0, else we wouldn't be here)
                    if ((_maxActive < 0 || pool.activeCount < _maxActive) &&
                        (_maxTotal < 0 || _totalActive + _totalIdle < _maxTotal)) {
                        // allow new object to be created
                    } else {
                        // the pool is exhausted
                        switch(_whenExhaustedAction) {
                            case WHEN_EXHAUSTED_GROW:
                                // allow new object to be created
                                break;
                            case WHEN_EXHAUSTED_FAIL:
                                throw new NoSuchElementException();
                            case WHEN_EXHAUSTED_BLOCK:
                                try {
                                    if(_maxWait <= 0) {
                                        wait();
                                    } else {
                                        // this code may be executed again after a notify
then continue cycle
                                        // so, need to calculate the amount of time to wait
                                        final long elapsed = (System.currentTimeMillis() -
starttime);
                                        final long waitTime = _maxWait - elapsed;
                                        if (waitTime > 0)
                                        {
                                            wait(waitTime);
                                        }
                                    }
                                } catch(InterruptedException e) {
                                    // ignored
                                }
                                if(_maxWait > 0 && ((System.currentTimeMillis()
- starttime) >= _maxWait)) {
                                    throw new NoSuchElementException("Timeout waiting for
idle object");
                                } else {
                                    continue; // keep looping
                                }
                            default:
                                throw new IllegalArgumentException("whenExhaustedAction "
+ _whenExhaustedAction + " not recognized.");
                        }
                    }
                }
                pool.incrementActiveCount();
            }
            
            // create new object when needed
            boolean newlyCreated = false;
            if(null == pair) {
                try {
                    Object obj = _factory.makeObject();
                    pair = new ObjectTimestampPair(obj);
                    newlyCreated = true;
                } finally {
                    if (!newlyCreated) {
                        // object cannot be created
                        synchronized (this) {
                            pool.decrementActiveCount();                            
                        }
                    }
                }
            }

            
            // Activate.  If activate fails, decrement active count and destroy.
            // If instance failing activation is new, throw NoSuchElementException;
            // otherwise keep looping
            try {
                _factory.activateObject(key, pair.value);
            } catch (Exception e) {
                try {
                    _factory.destroyObject(key,pair.value);
                    synchronized (this) {
                        pool.decrementActiveCount();
                    }
                } catch (Exception e2) {
                    // swallowed
                }
                if(newlyCreated) {
                    throw new NoSuchElementException(
                       "Could not create a validated object, cause: "
                            + e.getMessage());
                }
                else {
                    continue; // keep looping
                }
            }

            // Validate.  If validation fails, decrement active count and
            // destroy. If instance failing validation is new, throw
            // NoSuchElementException; otherwise keep looping
            boolean invalid = true;
            try {
                invalid = _testOnBorrow && !_factory.validateObject(key, pair.value);
            } catch (Exception e) {
                // swallowed
            }
            if (invalid) {
                try {
                    _factory.destroyObject(key,pair.value);
                    synchronized (this) {
                        pool.decrementActiveCount();
                    }
                } catch (Exception e) {
                    // swallowed
                }
                if(newlyCreated) {
                    throw new NoSuchElementException("Could not create a validated object");
                } // else keep looping
            } else {
                return pair.value;
            }
        }
    }

</pre>




> GenericKeyedObjectPool.borrowObject(Object key) blocked for all threads until _factory.makeObject(key)
is finished
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: POOL-132
>                 URL: https://issues.apache.org/jira/browse/POOL-132
>             Project: Commons Pool
>          Issue Type: Improvement
>    Affects Versions: 1.4
>            Reporter: Michael Mueller
>            Priority: Minor
>
> If no Object is in the pool the method GenericKeyedObjectPool.borrowObject(Object key)
is blocking until a new Object (_factory.makeObject(key) ) is created.  This will block all
other threads even if they want an Object for a different key or want return an Object.
> The GenericObjectPool.borrowObject() have a better implementation where the _factory.makeObject(key)
 is called after the synchronized block.
> Could this changed like this?
> {code:title=GenericKeyedObjectPool.borrowObject(Object key)|borderStyle=solid}
>     public Object borrowObject(Object key) throws Exception {
>         long starttime = System.currentTimeMillis();
>         
>         for(;;) {
>             ObjectTimestampPair pair = null;
>             ObjectQueue pool = null;
>             synchronized (this) {
>                 assertOpen();
>                 pool = (ObjectQueue)(_poolMap.get(key));
>                 if(null == pool) {
>                     pool = new ObjectQueue();
>                     _poolMap.put(key,pool);
>                     _poolList.add(key);
>                 }
>                 // if there are any sleeping, just grab one of those
>                 try {
>                     pair = (ObjectTimestampPair)(pool.queue.removeFirst());
>                     if(null != pair) {
>                         _totalIdle--;
>                     }
>                 } catch(NoSuchElementException e) { /* ignored */
>                 }
>                 // otherwise
>                 if(null == pair) {
>                     // if there is a totalMaxActive and we are at the limit then
>                     // we have to make room
>                     if ((_maxTotal > 0) && (_totalActive + _totalIdle >=
_maxTotal)) {
>                         clearOldest();
>                     }
>     
>                     // check if we can create one
>                     // (note we know that the num sleeping is 0, else we wouldn't be
here)
>                     if ((_maxActive < 0 || pool.activeCount < _maxActive) &&
>                         (_maxTotal < 0 || _totalActive + _totalIdle < _maxTotal))
{
>                         // allow new object to be created
>                     } else {
>                         // the pool is exhausted
>                         switch(_whenExhaustedAction) {
>                             case WHEN_EXHAUSTED_GROW:
>                                 // allow new object to be created
>                                 break;
>                             case WHEN_EXHAUSTED_FAIL:
>                                 throw new NoSuchElementException();
>                             case WHEN_EXHAUSTED_BLOCK:
>                                 try {
>                                     if(_maxWait <= 0) {
>                                         wait();
>                                     } else {
>                                         // this code may be executed again after a notify
then continue cycle
>                                         // so, need to calculate the amount of time to
wait
>                                         final long elapsed = (System.currentTimeMillis()
- starttime);
>                                         final long waitTime = _maxWait - elapsed;
>                                         if (waitTime > 0)
>                                         {
>                                             wait(waitTime);
>                                         }
>                                     }
>                                 } catch(InterruptedException e) {
>                                     // ignored
>                                 }
>                                 if(_maxWait > 0 && ((System.currentTimeMillis()
- starttime) >= _maxWait)) {
>                                     throw new NoSuchElementException("Timeout waiting
for idle object");
>                                 } else {
>                                     continue; // keep looping
>                                 }
>                             default:
>                                 throw new IllegalArgumentException("whenExhaustedAction
" + _whenExhaustedAction + " not recognized.");
>                         }
>                     }
>                 }
>                 pool.incrementActiveCount();
>             }
>             
>             // create new object when needed
>             boolean newlyCreated = false;
>             if(null == pair) {
>                 try {
>                     Object obj = _factory.makeObject();
>                     pair = new ObjectTimestampPair(obj);
>                     newlyCreated = true;
>                 } finally {
>                     if (!newlyCreated) {
>                         // object cannot be created
>                         synchronized (this) {
>                             pool.decrementActiveCount();                            
>                         }
>                     }
>                 }
>             }
>             
>             // Activate.  If activate fails, decrement active count and destroy.
>             // If instance failing activation is new, throw NoSuchElementException;
>             // otherwise keep looping
>             try {
>                 _factory.activateObject(key, pair.value);
>             } catch (Exception e) {
>                 try {
>                     _factory.destroyObject(key,pair.value);
>                     synchronized (this) {
>                         pool.decrementActiveCount();
>                     }
>                 } catch (Exception e2) {
>                     // swallowed
>                 }
>                 if(newlyCreated) {
>                     throw new NoSuchElementException(
>                        "Could not create a validated object, cause: "
>                             + e.getMessage());
>                 }
>                 else {
>                     continue; // keep looping
>                 }
>             }
>             // Validate.  If validation fails, decrement active count and
>             // destroy. If instance failing validation is new, throw
>             // NoSuchElementException; otherwise keep looping
>             boolean invalid = true;
>             try {
>                 invalid = _testOnBorrow && !_factory.validateObject(key, pair.value);
>             } catch (Exception e) {
>                 // swallowed
>             }
>             if (invalid) {
>                 try {
>                     _factory.destroyObject(key,pair.value);
>                     synchronized (this) {
>                         pool.decrementActiveCount();
>                     }
>                 } catch (Exception e) {
>                     // swallowed
>                 }
>                 if(newlyCreated) {
>                     throw new NoSuchElementException("Could not create a validated object");
>                 } // else keep looping
>             } else {
>                 return pair.value;
>             }
>         }
>     }
> {code} 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message