You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2013/01/10 06:32:48 UTC
svn commit: r1431191 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
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);
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...
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);
Re: svn commit: r1431191 -
/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectio
nFactory.java
Posted by 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);
Re: svn commit: r1431191 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/connection/DBCPConnectionFactory.java
Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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);
>
> 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?
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);
>
>
>