Re: svn commit: r1431191 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectio nFactory.java

[Jacques Le Roux <ja...@les7arts.com>]

Jacopo Cappellato wrote:
> Jacques,
> 
> please see inline:
> 
> On Jan 10, 2013, at 6:32 AM, jleroux@apache.org wrote:
> 
>> Author: jleroux
>> Date: Thu Jan 10 05:32:48 2013
>> New Revision: 1431191
>> 
>> URL: http://svn.apache.org/viewvc?rev=1431191&view=rev
>> Log:
>> In "Memory leak due to transaction management using DBCP and MySQL" https://issues.apache.org/jira/browse/OFBIZ-5122 Jose Manuel
>> Vivó Arnal reported that I missed to apply the patch from OFBIZ-2599, just handled the DBPC part, and Philippe Mouawad did not
>> notice.  
>> 
>> OK, so it was just a matter of replacing
>> PoolableConnectionFactory factory = new PoolableConnectionFactory(xacf, pool, null, null, true, true);
>> by
>> PoolableManagedConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
> 
> This is minor but it would be better to write:
> 
> PoolableConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);

OK, I can change that in trunk
 
>> 
>> The rest are only imports cleaning.
>> 
>> Too bad we did not notice that for 3.5 years. It seems it does not affect servers with plenty of memory, or updated often
>> enough. But I recommend to upgrade of course... 
> 
> Are we sure we can backport this fix to all the release branches as you did? From the history of comments in OFBIZ-2599 I see
> that there are some prerequisites for the version of DBCP: did you check if they are satisfied by the jars in 10.04 and 11.04? 

Yes, commons-dbcp-1.4.jar is present since R10.04 (also else it would not have compiled ;o)
The change has been introduced since 1.3: https://issues.apache.org/jira/browse/DBCP-294

Jacques


> Regards,
> 
> Jacopo
> 
>> 
>> Modified:
>>    ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
>> 
>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java?rev=1431191&r1=1431190&r2=1431191&view=diff
>> ============================================================================== ---
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java (original) +++
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java Thu Jan 10 05:32:48 2013 @@ -18,11
>>  +18,22 @@ *******************************************************************************/
>> package org.ofbiz.entity.connection;
>> 
>> +import java.sql.Connection;
>> +import java.sql.Driver;
>> +import java.sql.SQLException;
>> +import java.util.HashMap;
>> +import java.util.Map;
>> +import java.util.Properties;
>> +
>> +import javax.transaction.TransactionManager;
>> +
>> +import javolution.util.FastMap;
>> +
>> import org.apache.commons.dbcp.ConnectionFactory;
>> import org.apache.commons.dbcp.DriverConnectionFactory;
>> -import org.apache.commons.dbcp.PoolableConnectionFactory;
>> import org.apache.commons.dbcp.managed.LocalXAConnectionFactory;
>> import org.apache.commons.dbcp.managed.ManagedDataSource;
>> +import org.apache.commons.dbcp.managed.PoolableManagedConnectionFactory;
>> import org.apache.commons.dbcp.managed.XAConnectionFactory;
>> import org.apache.commons.pool.impl.GenericObjectPool;
>> import org.ofbiz.base.util.Debug;
>> @@ -32,16 +43,6 @@ import org.ofbiz.entity.datasource.Gener
>> import org.ofbiz.entity.transaction.TransactionFactory;
>> import org.w3c.dom.Element;
>> 
>> -import javax.transaction.TransactionManager;
>> -import java.sql.Connection;
>> -import java.sql.Driver;
>> -import java.sql.SQLException;
>> -import java.util.HashMap;
>> -import java.util.Map;
>> -import java.util.Properties;
>> -
>> -import javolution.util.FastMap;
>> -
>> /**
>>  * DBCPConnectionFactory
>>  */
>> @@ -146,7 +147,7 @@ public class DBCPConnectionFactory imple
>> 
>> 
>>             // create the pool object factory
>> -            PoolableConnectionFactory factory = new PoolableConnectionFactory(xacf, pool, null, null, true, true);
>> +            PoolableManagedConnectionFactory factory = new PoolableManagedConnectionFactory(xacf, pool, null, null, true, true);
>>             factory.setValidationQuery("select 1 from entity_key_store where key_name = ''");
>>             factory.setDefaultReadOnly(false);