You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2014/04/17 00:28:32 UTC
Re: svn commit: r1588087 - in /commons/proper/dbcp/trunk/src:
main/java/org/apache/commons/dbcp2/ test/java/org/apache/commons/dbcp2/
On 16 April 2014 23:12, <ps...@apache.org> wrote:
> Author: psteitz
> Date: Wed Apr 16 22:12:15 2014
> New Revision: 1588087
>
> URL: http://svn.apache.org/r1588087
> Log:
> Modified DelegatingStatement#close to a) null its delegate reference b) no-op on isClosed. JIRA: DBCP-415.
>
> Modified:
> commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java
> commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java
> commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java
>
> Modified: commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java?rev=1588087&r1=1588086&r2=1588087&view=diff
> ==============================================================================
> --- commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java (original)
> +++ commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java Wed Apr 16 22:12:15 2014
> @@ -27,6 +27,7 @@ import java.sql.Ref;
> import java.sql.ResultSet;
> import java.sql.ResultSetMetaData;
> import java.sql.SQLException;
> +import java.sql.Statement;
> import java.sql.Time;
> import java.sql.Timestamp;
> import java.util.Calendar;
> @@ -252,7 +253,8 @@ public class DelegatingPreparedStatement
> */
> @Override
> public String toString() {
> - return getDelegate().toString();
> + Statement statement = getDelegate();
> + return statement == null ? "NULL" : getDelegate().toString();
Why call getDelegate() again? I would expect to see:
return statement == null ? "NULL" : statement .toString();
Otherwise could get NPE if getDelegate() changes to return null ...
> }
>
> @Override
>
> Modified: commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java?rev=1588087&r1=1588086&r2=1588087&view=diff
> ==============================================================================
> --- commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java (original)
> +++ commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java Wed Apr 16 22:12:15 2014
> @@ -129,6 +129,9 @@ public class DelegatingStatement extends
> */
> @Override
> public void close() throws SQLException {
> + if (isClosed()) {
> + return;
> + }
> try {
> try {
> if (_conn != null) {
> @@ -159,6 +162,7 @@ public class DelegatingStatement extends
> }
> finally {
> _closed = true;
> + _stmt = null;
> }
> }
>
> @@ -352,7 +356,7 @@ public class DelegatingStatement extends
> */
> @Override
> public String toString() {
> - return _stmt.toString();
> + return _stmt == null ? "NULL" : _stmt.toString();
> }
>
> @Override
>
> Modified: commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java?rev=1588087&r1=1588086&r2=1588087&view=diff
> ==============================================================================
> --- commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java (original)
> +++ commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java Wed Apr 16 22:12:15 2014
> @@ -155,7 +155,7 @@ public class TestPStmtPoolingBasicDataSo
>
> conn.setCatalog("catalog1");
> DelegatingPreparedStatement stmt3 = (DelegatingPreparedStatement) conn.prepareStatement("select 'a' from dual");
> - TesterPreparedStatement inner3 = (TesterPreparedStatement) stmt1.getInnermostDelegate();
> + TesterPreparedStatement inner3 = (TesterPreparedStatement) stmt3.getInnermostDelegate();
> assertEquals("catalog1", inner3.getCatalog());
> stmt3.close();
>
> @@ -231,7 +231,7 @@ public class TestPStmtPoolingBasicDataSo
>
> // currently fails with AssertionFailedError: Did not expect any threads to fail expected:<0> but was:<1>
> // The following appears in the console: Unexpected error: ResultSet is closed.
> - public void IGNOREDtestMultipleThreads1() throws Exception {
> + public void testMultipleThreads1() throws Exception {
> ds.setMaxWaitMillis(-1);
> ds.setMaxTotal(5);
> ds.setMaxOpenPreparedStatements(-1);
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1588087 - in /commons/proper/dbcp/trunk/src: main/java/org/apache/commons/dbcp2/
test/java/org/apache/commons/dbcp2/
Posted by Phil Steitz <ph...@gmail.com>.
On 4/16/14, 3:28 PM, sebb wrote:
> On 16 April 2014 23:12, <ps...@apache.org> wrote:
>> Author: psteitz
>> Date: Wed Apr 16 22:12:15 2014
>> New Revision: 1588087
>>
>> URL: http://svn.apache.org/r1588087
>> Log:
>> Modified DelegatingStatement#close to a) null its delegate reference b) no-op on isClosed. JIRA: DBCP-415.
>>
>> Modified:
>> commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java
>> commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java
>> commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java
>>
>> Modified: commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java
>> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java?rev=1588087&r1=1588086&r2=1588087&view=diff
>> ==============================================================================
>> --- commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java (original)
>> +++ commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingPreparedStatement.java Wed Apr 16 22:12:15 2014
>> @@ -27,6 +27,7 @@ import java.sql.Ref;
>> import java.sql.ResultSet;
>> import java.sql.ResultSetMetaData;
>> import java.sql.SQLException;
>> +import java.sql.Statement;
>> import java.sql.Time;
>> import java.sql.Timestamp;
>> import java.util.Calendar;
>> @@ -252,7 +253,8 @@ public class DelegatingPreparedStatement
>> */
>> @Override
>> public String toString() {
>> - return getDelegate().toString();
>> + Statement statement = getDelegate();
>> + return statement == null ? "NULL" : getDelegate().toString();
> Why call getDelegate() again? I would expect to see:
>
> return statement == null ? "NULL" : statement .toString();
Thanks! That's what I meant :)
Fixed in r1588090
Phil
>
> Otherwise could get NPE if getDelegate() changes to return null ...
>
>> }
>>
>> @Override
>>
>> Modified: commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java
>> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java?rev=1588087&r1=1588086&r2=1588087&view=diff
>> ==============================================================================
>> --- commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java (original)
>> +++ commons/proper/dbcp/trunk/src/main/java/org/apache/commons/dbcp2/DelegatingStatement.java Wed Apr 16 22:12:15 2014
>> @@ -129,6 +129,9 @@ public class DelegatingStatement extends
>> */
>> @Override
>> public void close() throws SQLException {
>> + if (isClosed()) {
>> + return;
>> + }
>> try {
>> try {
>> if (_conn != null) {
>> @@ -159,6 +162,7 @@ public class DelegatingStatement extends
>> }
>> finally {
>> _closed = true;
>> + _stmt = null;
>> }
>> }
>>
>> @@ -352,7 +356,7 @@ public class DelegatingStatement extends
>> */
>> @Override
>> public String toString() {
>> - return _stmt.toString();
>> + return _stmt == null ? "NULL" : _stmt.toString();
>> }
>>
>> @Override
>>
>> Modified: commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java
>> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java?rev=1588087&r1=1588086&r2=1588087&view=diff
>> ==============================================================================
>> --- commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java (original)
>> +++ commons/proper/dbcp/trunk/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java Wed Apr 16 22:12:15 2014
>> @@ -155,7 +155,7 @@ public class TestPStmtPoolingBasicDataSo
>>
>> conn.setCatalog("catalog1");
>> DelegatingPreparedStatement stmt3 = (DelegatingPreparedStatement) conn.prepareStatement("select 'a' from dual");
>> - TesterPreparedStatement inner3 = (TesterPreparedStatement) stmt1.getInnermostDelegate();
>> + TesterPreparedStatement inner3 = (TesterPreparedStatement) stmt3.getInnermostDelegate();
>> assertEquals("catalog1", inner3.getCatalog());
>> stmt3.close();
>>
>> @@ -231,7 +231,7 @@ public class TestPStmtPoolingBasicDataSo
>>
>> // currently fails with AssertionFailedError: Did not expect any threads to fail expected:<0> but was:<1>
>> // The following appears in the console: Unexpected error: ResultSet is closed.
>> - public void IGNOREDtestMultipleThreads1() throws Exception {
>> + public void testMultipleThreads1() throws Exception {
>> ds.setMaxWaitMillis(-1);
>> ds.setMaxTotal(5);
>> ds.setMaxOpenPreparedStatements(-1);
>>
>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org