Finished implementation of forceSynchronousCheckins parameter.

BasicResourcePool.checkinResource(...) is no longer sync'ed, and
expects to be called without the pools lock, so that long
refurbishments in a synchronized checkin can be performed without
holding the pool's lock.

src/codegen/com/mchange/v2/c3p0/impl/WrapperConnectionPoolDataSourceBase.beangen-xml

@@ -166,6 +166,13 @@
        <getter><modifiers><modifier>public</modifier><modifier>synchronized</modifier></modifiers></getter>
        <setter><modifiers><modifier>public</modifier><modifier>synchronized</modifier></modifiers></setter>
     </property>
+    <property>
+       <type>boolean</type>
+       <name>forceSynchronousCheckins</name>
+       <default-value>C3P0Config.initializeBooleanPropertyVar("forceSynchronousCheckins", C3P0Defaults.forceSynchronousCheckins())</default-value>
+       <getter><modifiers><modifier>public</modifier><modifier>synchronized</modifier></modifiers></getter>
+       <setter><modifiers><modifier>public</modifier><modifier>synchronized</modifier></modifiers></setter>
+    </property>
     <property>
        <type>boolean</type>
        <name>testConnectionOnCheckout</name>

src/java/com/mchange/v2/c3p0/AbstractComboPooledDataSource.java

@@ -694,6 +694,18 @@ public void setDebugUnreturnedConnectionStackTraces(boolean debugUnreturnedConne
 	}
     }
 
+    public boolean isForceSynchronousCheckins()
+    { return wcpds.isForceSynchronousCheckins(); }
+
+    public void setForceSynchronousCheckins(boolean forceSynchronousCheckins)
+    {
+	if ( diff( wcpds.isForceSynchronousCheckins(), forceSynchronousCheckins ) )
+	{
+	    wcpds.setForceSynchronousCheckins( forceSynchronousCheckins ); 
+	    this.resetPoolManager( false );
+	}
+    }
+
     public int getStatementCacheNumDeferredCloseThreads()
     { return wcpds.getStatementCacheNumDeferredCloseThreads(); }
 

src/java/com/mchange/v2/c3p0/impl/C3P0Defaults.java

@@ -71,6 +71,7 @@ public final class C3P0Defaults
     private final static boolean DEBUG_UNRETURNED_CONNECTION_STACK_TRACES    = false;
     private final static boolean PRIVILEGE_SPAWNED_THREADS                   = false;
     private final static boolean FORCE_USE_NAMED_DRIVER_CLASS                = false;
+    private final static boolean FORCE_SYNCHRONOUS_CHECKINS                  = false;
 
     private final static int NUM_HELPER_THREADS = 3;
 
@@ -254,6 +255,9 @@ public static int maxConnectionAge()
     public static String dataSourceName()
     { return DATA_SOURCE_NAME; }
 
+    public static boolean forceSynchronousCheckins()
+    { return FORCE_SYNCHRONOUS_CHECKINS; }
+
     public static Map extensions()
     { return EXTENSIONS; }
 }

src/java/com/mchange/v2/c3p0/impl/C3P0PooledConnectionPool.java

@@ -147,6 +147,7 @@ public Object getInUseLock(Object resc)
                     int propertyCycle, //seconds
                     int unreturnedConnectionTimeout, //seconds
                     boolean debugUnreturnedConnectionStackTraces,
+                    boolean forceSynchronousCheckins,
                     final boolean testConnectionOnCheckout,
                     final boolean testConnectionOnCheckin,
                     int maxStatements,
@@ -665,6 +666,7 @@ public void destroyResource(Object resc, boolean checked_out) throws Exception
                 fact.setExpirationEnforcementDelay( propertyCycle * 1000 );
                 fact.setDestroyOverdueResourceTime( unreturnedConnectionTimeout * 1000 );
                 fact.setDebugStoreCheckoutStackTrace( debugUnreturnedConnectionStackTraces );
+                fact.setForceSynchronousCheckins( forceSynchronousCheckins );
                 fact.setAcquisitionRetryAttempts( acq_retry_attempts );
                 fact.setAcquisitionRetryDelay( acq_retry_delay );
                 fact.setBreakOnAcquisitionFailure( break_after_acq_failure );
@@ -673,7 +675,7 @@ public void destroyResource(Object resc, boolean checked_out) throws Exception
         }
         catch (ResourcePoolException e)
         { throw SqlUtils.toSQLException(e); }
-                    }
+    }
 
     public PooledConnection checkoutPooledConnection() throws SQLException
     { 
@@ -1022,10 +1024,7 @@ public int getNumIdleConnections() throws SQLException
     public int getNumBusyConnections() throws SQLException
     { 
         try 
-        {
-            synchronized ( rp )
-            { return (rp.getAwaitingCheckinCount() - rp.getExcludedCount()); }
-        }
+        { return rp.getAwaitingCheckinNotExcludedCount(); }
         catch ( Exception e )
         { 
             //e.printStackTrace();

src/java/com/mchange/v2/c3p0/impl/C3P0PooledConnectionPoolManager.java

@@ -801,6 +801,17 @@ private boolean getDebugUnreturnedConnectionStackTraces(String userName)
         }
     }
 
+    private boolean getForceSynchronousCheckins(String userName)
+    {
+        try
+        { return getBoolean("forceSynchronousCheckins", userName ); }
+        catch (Exception e)
+        {
+            if ( logger.isLoggable( MLevel.FINE ) )
+                logger.log( MLevel.FINE, "Could not fetch boolean property", e);
+            return C3P0Defaults.forceSynchronousCheckins();
+        }
+    }
 
     private String getConnectionTesterClassName(String userName)
     { return getString("connectionTesterClassName", userName ); }
@@ -966,6 +977,7 @@ private C3P0PooledConnectionPool createPooledConnectionPool(DbAuth auth) throws
 								      this.getPropertyCycle( userName ),
 								      this.getUnreturnedConnectionTimeout( userName ),
 								      this.getDebugUnreturnedConnectionStackTraces( userName ),
+								      this.getForceSynchronousCheckins( userName ),
 								      this.getTestConnectionOnCheckout( userName ),
 								      this.getTestConnectionOnCheckin( userName ),
 								      this.getMaxStatements( userName ),

src/java/com/mchange/v2/c3p0/jboss/C3P0PooledDataSource.java

@@ -473,6 +473,9 @@ public int getUnreturnedConnectionTimeout()
     public boolean isDebugUnreturnedConnectionStackTraces()
     { return combods.isDebugUnreturnedConnectionStackTraces(); }
 
+    public boolean isForceSynchronousCheckins()
+    { return combods.isForceSynchronousCheckins(); }
+
     public void setConnectionCustomizerClassName(String connectionCustomizerClassName) throws NamingException
     {
         combods.setConnectionCustomizerClassName(connectionCustomizerClassName);
@@ -485,6 +488,12 @@ public void setDebugUnreturnedConnectionStackTraces(boolean debugUnreturnedConne
         rebind();
     }
 
+    public void setForceSynchronousCheckins(boolean forceSynchronousCheckins) throws NamingException
+    {
+        combods.setForceSynchronousCheckins(forceSynchronousCheckins);
+        rebind();
+    }
+
     public void setMaxAdministrativeTaskTime(int maxAdministrativeTaskTime) throws NamingException
     {
         combods.setMaxAdministrativeTaskTime(maxAdministrativeTaskTime);

src/java/com/mchange/v2/c3p0/jboss/C3P0PooledDataSourceMBean.java

@@ -76,6 +76,9 @@ public interface C3P0PooledDataSourceMBean
 
     public boolean isDebugUnreturnedConnectionStackTraces();
     public void setDebugUnreturnedConnectionStackTraces(boolean debugUnreturnedConnectionStackTraces) throws NamingException;
+
+    public boolean isForceSynchronousCheckins();
+    public void setForceSynchronousCheckins(boolean forceSynchronousCheckins) throws NamingException;
 
     public String getConnectionCustomizerClassName();
     public void setConnectionCustomizerClassName( String connectionCustomizerClassName ) throws NamingException;

src/java/com/mchange/v2/resourcepool/BasicResourcePool.java

@@ -731,54 +731,57 @@ else if (logger.isLoggable( MLevel.INFO ))
 	}
     }
 
-    public synchronized void checkinResource( Object resc ) 
-    throws ResourcePoolException
+    public void checkinResource( Object resc ) throws ResourcePoolException
     {
-        try
-        {
-            //we permit straggling resources to be checked in 
-            //without exception even if we are broken
-            if (managed.keySet().contains(resc))
-                doCheckinManaged( resc );
-            else if (excluded.contains(resc))
-                doCheckinExcluded( resc );
-            else if ( isFormerResource(resc) )
-            {
-                if ( logger.isLoggable( MLevel.FINER ) )
-                    logger.finer("Resource " + resc + " checked-in after having been checked-in already, or checked-in after " +
-                    " having being destroyed for being checked-out too long.");
-            }
-            else
-                throw new ResourcePoolException("ResourcePool" + (broken ? " [BROKEN!]" : "") + ": Tried to check-in a foreign resource!");
-            if (Debug.DEBUG && Debug.TRACE == Debug.TRACE_MAX) trace();
-        }
-        catch ( ResourceClosedException e ) // one of our async threads died
-        {
-//          System.err.println(this + 
-//          " - checkinResource( ... ) -- even broken pools should allow checkins without exception. probable resource pool bug.");
-//          e.printStackTrace();
-
-            if ( logger.isLoggable( MLevel.SEVERE ) )
-                logger.log( MLevel.SEVERE, 
-                                this + " - checkinResource( ... ) -- even broken pools should allow checkins without exception. probable resource pool bug.", 
-                                e);
+	try
+	{
+	    boolean unlocked_do_checkin_managed = false;
+	    synchronized ( this )
+	    {    
+		//we permit straggling resources to be checked in 
+		//without exception even if we are broken
+		if (managed.keySet().contains(resc))
+		    unlocked_do_checkin_managed = true;
+		else if (excluded.contains(resc))
+		    doCheckinExcluded( resc );
+		else if ( isFormerResource(resc) )
+		    {
+			if ( logger.isLoggable( MLevel.FINER ) )
+			    logger.finer("Resource " + resc + " checked-in after having been checked-in already, or checked-in after " +
+					 " having being destroyed for being checked-out too long.");
+		    }
+		else
+		    throw new ResourcePoolException("ResourcePool" + (broken ? " [BROKEN!]" : "") + ": Tried to check-in a foreign resource!");
+	    }
+	    if ( unlocked_do_checkin_managed ) doCheckinManaged( resc );
+	    if (Debug.DEBUG && Debug.TRACE == Debug.TRACE_MAX) syncTrace();
+	}
+	catch ( ResourceClosedException e ) // one of our async threads died
+	{
+	    if ( logger.isLoggable( MLevel.SEVERE ) )
+		logger.log( MLevel.SEVERE, 
+			    this + " - checkinResource( ... ) -- even broken pools should allow checkins without exception. probable resource pool bug.", 
+			    e);
 
-            this.unexpectedBreak();
-            throw e;
-        }
+	    this.unexpectedBreak();
+	    throw e;
+	}
     }
 
-    public synchronized void checkinAll()
-    throws ResourcePoolException
+    public void checkinAll() throws ResourcePoolException
     {
         try
         {
-            Set checkedOutNotExcluded = new HashSet( managed.keySet() );
-            checkedOutNotExcluded.removeAll( unused );
-            for (Iterator ii = checkedOutNotExcluded.iterator(); ii.hasNext(); )
-                doCheckinManaged( ii.next() );
-            for (Iterator ii = excluded.iterator(); ii.hasNext(); )
-                doCheckinExcluded( ii.next() );
+            Set checkedOutNotExcluded = null;
+	    synchronized ( this ) 
+	    {
+		checkedOutNotExcluded = new HashSet( managed.keySet() );
+		checkedOutNotExcluded.removeAll( unused );
+		for (Iterator ii = excluded.iterator(); ii.hasNext(); )
+		    doCheckinExcluded( ii.next() );
+	    }
+	    for (Iterator ii = checkedOutNotExcluded.iterator(); ii.hasNext(); )
+		doCheckinManaged( ii.next() );
         }
         catch ( ResourceClosedException e ) // one of our async threads died
         {
@@ -872,6 +875,9 @@ public synchronized int getExcludedCount()
     public synchronized int getAwaitingCheckinCount()
     { return managed.size() - unused.size() + excluded.size(); }
 
+    public synchronized int getAwaitingCheckinNotExcludedCount()
+    { return managed.size() - unused.size(); }
+
     public synchronized void resetPool()
     {
         try
@@ -1347,67 +1353,61 @@ public void run()
     //debug only
     //Map lastCheckIns = Collections.synchronizedMap( new HashMap() );
 
+    // we insist the pool's lock not be held to avoid refurbishment on checkin with
+    // the lock if synchronous checkins have been forced
     private void doCheckinManaged( final Object resc ) throws ResourcePoolException
     {
-        assert Thread.holdsLock( this );
-
-
+        assert !Thread.holdsLock( this );
 
-        if (unused.contains(resc))
-        {
-            if ( Debug.DEBUG )
-		{
-		    //Exception e = (Exception) lastCheckIns.get( resc );
-		    //e.printStackTrace();
-		    //throw new ResourcePoolException("Tried to check-in an already checked-in resource: " + resc, (Exception) lastCheckIns.get( resc ));
+        if ( Debug.DEBUG && this.statusInPool( resc ) == KNOWN_AND_AVAILABLE )
+	    throw new ResourcePoolException("Tried to check-in an already checked-in resource: " + resc);
 
-		    throw new ResourcePoolException("Tried to check-in an already checked-in resource: " + resc);
-		}
+	synchronized ( this )
+	{
+          if (broken)
+	  {
+	      removeResource( resc, true ); //synchronous... if we're broken, async tasks might not work
+	      return;
+	  }
         }
-        else if (broken)
-            removeResource( resc, true ); //synchronous... if we're broken, async tasks might not work
-        else
-        {
-            class RefurbishCheckinResourceTask implements Runnable
-            {
-                public void run()
+
+	class RefurbishCheckinResourceTask implements Runnable
+	{
+	    public void run()
+	    {
+		boolean resc_okay = attemptRefurbishResourceOnCheckin( resc );
+		synchronized( BasicResourcePool.this )
                 {
-                    boolean resc_okay = attemptRefurbishResourceOnCheckin( resc );
-                    synchronized( BasicResourcePool.this )
+		    PunchCard card = (PunchCard) managed.get( resc );
+
+		    if ( resc_okay && card != null) //we have to check that the resource is still in the pool
                     {
-                        PunchCard card = (PunchCard) managed.get( resc );
-
-                        if ( resc_okay && card != null) //we have to check that the resource is still in the pool
-                        {
-                            unused.add(0,  resc );
+			unused.add(0,  resc );
 
-                            card.last_checkin_time = System.currentTimeMillis();
-                            card.checkout_time = -1;
-                        }
-                        else
-                        {
-                            if (card != null)
-                                card.checkout_time = -1; //so we don't see this as still checked out and log an overdue cxn in removeResource()
-
-                            removeResource( resc );
-                            ensureMinResources();
+			card.last_checkin_time = System.currentTimeMillis();
+			card.checkout_time = -1;
+                    }
+		    else
+                    {
+			if (card != null)
+			    card.checkout_time = -1; //so we don't see this as still checked out and log an overdue cxn in removeResource()
 
-                            if (card == null && logger.isLoggable( MLevel.FINE ))
-                                logger.fine("Resource " + resc + " was removed from the pool during its refurbishment for checkin.");
-                        }
+			removeResource( resc );
+			ensureMinResources();
 
-                        asyncFireResourceCheckedIn( resc, managed.size(), unused.size(), excluded.size() );
-                        BasicResourcePool.this.notifyAll();
+			if (card == null && logger.isLoggable( MLevel.FINE ))
+			    logger.fine("Resource " + resc + " was removed from the pool during its refurbishment for checkin.");
                     }
-                }
-            }
-
-            Runnable doMe = new RefurbishCheckinResourceTask();
-	    if ( force_synchronous_checkins ) doMe.run();
-            else taskRunner.postRunnable( doMe );
-        }
+
+		    asyncFireResourceCheckedIn( resc, managed.size(), unused.size(), excluded.size() );
+		    BasicResourcePool.this.notifyAll();
+		}
+	    }
+	}
 
-	//lastCheckIns.put( resc, new Exception("LAST CHECK IN") );
+	Runnable doMe = new RefurbishCheckinResourceTask();
+	if ( force_synchronous_checkins ) doMe.run();
+	else taskRunner.postRunnable( doMe );
     }
 
     private void doCheckinExcluded( Object resc )
@@ -1786,6 +1786,9 @@ private void ensureNotBroken() throws ResourcePoolException
             throw new ResourcePoolException("Attempted to use a closed or broken resource pool");
     }
 
+    private synchronized void syncTrace()
+    { trace(); }
+
     private void trace()
     {
         assert Thread.holdsLock( this );

src/java/com/mchange/v2/resourcepool/ResourcePool.java

@@ -88,6 +88,9 @@ public int getExcludedCount()
     public int getAwaitingCheckinCount()
 	throws ResourcePoolException;
 
+    public int getAwaitingCheckinNotExcludedCount()
+	throws ResourcePoolException;
+
     public long getEffectiveExpirationEnforcementDelay()
     throws ResourcePoolException;
 

src/test-properties/c3p0.properties

@@ -19,6 +19,7 @@
 #c3p0.numHelperThreads=10
 #c3p0.unreturnedConnectionTimeout=15
 #c3p0.debugUnreturnedConnectionStackTraces=true
+#c3p0.forceSynchronousCheckins=true
 #c3p0.maxStatements=20
 #c3p0.maxStatementsPerConnection=5
 #c3p0.maxAdministrativeTaskTime=3