[
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.
