commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ku...@gmx.de
Subject Re: [POOL] Offer of help for a 1.4 release
Date Sat, 05 Jan 2008 14:49:58 GMT
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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message