Hi Phil,
thanks for taking my issue serious.
Phil Steitz wrote:
Thanks again for the feedback. Any other opinions / suggestions on
this are appreciated. I suggest the following compromise, which would
also fix the maxActive exceeded by one issue I discovered with
1.2/1.4-RC1 yesterday:
(*) Move makeObject back inside synchronized scope in borrowObject.
(patch below)
This addresses Christoph's production use case (assuming I have
understood it correctly) and also closes the maxActive exceeded by one
problem that my testing uncovered yesterday. It does not have a huge
performance impact in the tests that I have run and still leaves
activation and validation (the per-request operations) outside of
synchronized scope. More elegant solutions to both problems are
certainly possible and we can consider them in subsequent releases -
including configurable serialization of just the makes.
This is not what I was trying to achieve :-(
While this fixes my issue with the parallel creation of objects, it
reopens another issue which I have with the implementation in pool 1.3:
The pool blocks completely while an object creation is in progress.
Just consider the following use case:
- Thread A tries to borrow an object. The pool contains no idle objects,
hence a new one is created
- While object is still in creation, Thread B tries to return his
borrowed object to the pool. Since returning the object depends on the
same synchronization lock as makeObject, B will block as long as the new
object isn't created (which can maybe take several seconds, as seen in
my use case). I think this is unacceptable.
- Still while the creation is in progress, Thread C wants to borrow an
object. It cannot continue, either, though an idle object would be
available, if B could have returned its object
I propose, as in my previous post, a distinct lock for object creation.
I have attached a patch which should fix this behavior, but probably
will not fix the "maxActive exceeded by 1". Do you already have a
testcase for this one? The existing unit tests run without errors when
my patch is applied.
Greetings
Christoph
Index: src/java/org/apache/commons/pool/impl/GenericObjectPool.java
===================================================================
--- src/java/org/apache/commons/pool/impl/GenericObjectPool.java
(revision 609143)
+++ src/java/org/apache/commons/pool/impl/GenericObjectPool.java
(working copy)
@@ -322,6 +322,8 @@
*/
public static final long
DEFAULT_SOFT_MIN_EVICTABLE_IDLE_TIME_MILLIS = -1;
+ private final Object makeObjectLock = new Object();
+
//--- constructors -----------------------------------------------
/**
@@ -920,7 +922,7 @@
} catch(NoSuchElementException e) {
; /* ignored */
}
-
+
// otherwise
if(null == pair) {
// check if we can create one
@@ -969,19 +971,21 @@
// 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) {
- _numActive--;
- notifyAll();
- }
- }
- }
+ synchronized( makeObjectLock ) {
+ try {
+ Object obj = _factory.makeObject();
+ pair = new ObjectTimestampPair(obj);
+ newlyCreated = true;
+ } finally {
+ if (!newlyCreated) {
+ // object cannot be created
+ synchronized (this) {
+ _numActive--;
+ notifyAll();
+ }
+ }
+ }
+ }
}
// activate & validate the object
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
