FAQ
Thanks for the greeting :) I'll try to partly return to the community
and maybe contribute something valuable, though I can't produce
anywhere near past activity these days..

2011/2/16 Andrus Adamchik <andrus@objectstyle.org>:
Andrey is back! :-)
On Feb 16, 2011, at 10:56 AM, andrey@apache.org wrote:

Author: andrey
Date: Wed Feb 16 08:56:55 2011
New Revision: 1071175

URL: http://svn.apache.org/viewvc?rev=1071175&view=rev
Log:
Plugging some potential resource leaks

Modified:
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/dbsync/ThrowOnPartialSchemaStrategy.java
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SelectAction.java
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/oracle/Oracle8SQLTemplateAction.java
cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/sqlite/SQLiteSQLTemplateAction.java
cayenne/main/trunk/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/graph/action/SaveAsImageAction.java

Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/dbsync/ThrowOnPartialSchemaStrategy.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/dbsync/ThrowOnPartialSchemaStrategy.java?rev=1071175&r1=1071174&r2=1071175&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/dbsync/ThrowOnPartialSchemaStrategy.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/dbsync/ThrowOnPartialSchemaStrategy.java Wed Feb 16 08:56:55 2011
@@ -25,6 +25,7 @@ import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+
import org.apache.cayenne.CayenneRuntimeException;
import org.apache.cayenne.access.DataNode;
import org.apache.cayenne.map.DbEntity;
@@ -48,21 +49,27 @@ public class ThrowOnPartialSchemaStrateg

List<String> schemas = new ArrayList<String>();
DatabaseMetaData md = null;
+        Connection connection = null;
try {
-            Connection connection = dataNode.getDataSource().getConnection();
-            md = connection.getMetaData();
-            ResultSet rs = md.getSchemas();
-
+            connection = dataNode.getDataSource().getConnection();
+
try {
-                while (rs.next()) {
-                    String schemaName = rs.getString(1);
-                    schemas.add(schemaName);
+                md = connection.getMetaData();
+                ResultSet rs = md.getSchemas();
+
+                try {
+                    while (rs.next()) {
+                        String schemaName = rs.getString(1);
+                        schemas.add(schemaName);
+                    }
+                }
+                finally {
+                    rs.close();
}
}
finally {
-                rs.close();
+                connection.close();
}
-            connection.close();
analyzer.analyzeSchemas(schemas, md);
}
catch (Exception e) {

Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java?rev=1071175&r1=1071174&r2=1071175&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java Wed Feb 16 08:56:55 2011
@@ -197,9 +197,7 @@ public class SQLTemplateAction implement
}
}
finally {
-            if (!iteratedResult) {
-                statement.close();
-            }
+            statement.close();
}
}


Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SelectAction.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SelectAction.java?rev=1071175&r1=1071174&r2=1071175&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SelectAction.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/access/jdbc/SelectAction.java Wed Feb 16 08:56:55 2011
@@ -68,7 +68,16 @@ public class SelectAction extends BaseSQ

SelectTranslator translator = createTranslator(connection);
PreparedStatement prepStmt = translator.createStatement();
-        ResultSet rs = prepStmt.executeQuery();
+        ResultSet rs;
+
+        // need to run in try-catch block to close statement properly if exception happens
+        try {
+            rs = prepStmt.executeQuery();
+        }
+        catch (Exception ex) {
+            prepStmt.close();
+            throw ex;
+        }
QueryMetadata md = query.getMetaData(getEntityResolver());
RowDescriptor descriptor = new RowDescriptorBuilder().setColumns(
translator.getResultColumns()).getDescriptor(

Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java?rev=1071175&r1=1071174&r2=1071175&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java Wed Feb 16 08:56:55 2011
@@ -58,9 +58,10 @@ public class DerbyPkGenerator extends Jd
}

Connection c = node.getDataSource().getConnection();
+        PreparedStatement select = null;

try {
-            PreparedStatement select = c.prepareStatement(
+            select = c.prepareStatement(
SELECT_QUERY,
ResultSet.TYPE_FORWARD_ONLY,
ResultSet.CONCUR_UPDATABLE);
@@ -91,6 +92,9 @@ public class DerbyPkGenerator extends Jd
return nextId;
}
finally {
+            if (select != null) {
+                select.close();
+            }
c.close();
}
}

Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/oracle/Oracle8SQLTemplateAction.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/oracle/Oracle8SQLTemplateAction.java?rev=1071175&r1=1071174&r2=1071175&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/oracle/Oracle8SQLTemplateAction.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/oracle/Oracle8SQLTemplateAction.java Wed Feb 16 08:56:55 2011
@@ -95,9 +95,7 @@ class Oracle8SQLTemplateAction extends S
// end - code different from super
}
finally {
-            if (!iteratedResult) {
-                statement.close();
-            }
+            statement.close();
}
}
}

Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/sqlite/SQLiteSQLTemplateAction.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/sqlite/SQLiteSQLTemplateAction.java?rev=1071175&r1=1071174&r2=1071175&view=diff
==============================================================================
--- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/sqlite/SQLiteSQLTemplateAction.java (original)
+++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/sqlite/SQLiteSQLTemplateAction.java Wed Feb 16 08:56:55 2011
@@ -91,9 +91,7 @@ class SQLiteSQLTemplateAction extends SQ
// end - code different from super
}
finally {
-            if (!iteratedResult) {
-                statement.close();
-            }
+            statement.close();
}
}


Modified: cayenne/main/trunk/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/graph/action/SaveAsImageAction.java
URL: http://svn.apache.org/viewvc/cayenne/main/trunk/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/graph/action/SaveAsImageAction.java?rev=1071175&r1=1071174&r2=1071175&view=diff
==============================================================================
--- cayenne/main/trunk/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/graph/action/SaveAsImageAction.java (original)
+++ cayenne/main/trunk/modeler/cayenne-modeler/src/main/java/org/apache/cayenne/modeler/graph/action/SaveAsImageAction.java Wed Feb 16 08:56:55 2011
@@ -74,19 +74,25 @@ public class SaveAsImageAction extends C
int status = chooser.showSaveDialog(Application.getFrame());
if (status == JFileChooser.APPROVE_OPTION) {
lastDir.updateFromChooser(chooser);
-            try {
-                String path = chooser.getSelectedFile().getPath();
-                if (!path.endsWith("." + ext)) {
-                    path += "." + ext;
-                }
-
+
+            String path = chooser.getSelectedFile().getPath();
+            if (!path.endsWith("." + ext)) {
+                path += "." + ext;
+            }
+
+            try {
OutputStream out = new FileOutputStream(path);

JGraph graph = dataDomainGraphTab.getGraph();
BufferedImage img = graph.getImage(null, 0);
-                ImageIO.write(img, ext, out);
-                out.flush();
-                out.close();
+
+                try {
+                    ImageIO.write(img, ext, out);
+                    out.flush();
+                }
+                finally {
+                    out.close();
+                }
}
catch (IOException ex) {
logObj.error("Could not save image", ex);



--
Andrey

Search Discussions

  • Andrey Razumovsky at Feb 20, 2011 at 8:39 pm
    Thanks for looking into this,

    I was out of the codebase for a while, and definitely could write down
    something meaningless.
    For this particular case, obviously close() inside try {} should be
    removed. This is something slipped from my vision when I looked into
    this. Luckily, JDBC javadoc says close() on closed statement does
    nothing, so likely that's why the tests didn't fail :)
    The reason I looked at this is that I have a tool, which shows
    possible connection leaks, and shows a sequence that leads into it. I
    can confirm, that one these files leak could *possibly* happen.
    As for nested try{}, I found this way is ugly, but the only one
    semantically correct. I used this way in ThrowOnPartialSchemaStrategy
    change in same changeset. The main reason of its necessity is that
    close() on any JDBC resources can produce SQLException itself, so all
    resources cannot be handled in same finally {}.
    I'll reconsider changes in DerbyPkGenerator ASAP (no doubt for me that
    the changes were required though).

    Thanks,
    Andrey

    2011/2/20 Andrus Adamchik <andrus@objectstyle.org>:
    I don't think this particular patch is correct. First of all it calls "select.close()" twice, second nesting of close operations is suspect (select.close() after c.commit()).

    On the general semantics of JDBC code. What are are normally using in Cayenne (with the exception of a few overlooked cases like this one), is a nested set of try { } finally {} over ResultSet, Statement, Connection. This ensures that "close" is called in the right sequence and regardless of the exception state. Also this seems to work pretty well (no bugs were reported that I can attribute to this code).

    Andrus
    On Feb 16, 2011, at 10:56 AM, andrey@apache.org wrote:
    Modified: cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java
    URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java?rev=1071175&r1=1071174&r2=1071175&view=diff
    ==============================================================================
    --- cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java (original)
    +++ cayenne/main/trunk/framework/cayenne-jdk1.5-unpublished/src/main/java/org/apache/cayenne/dba/derby/DerbyPkGenerator.java Wed Feb 16 08:56:55 2011
    @@ -58,9 +58,10 @@ public class DerbyPkGenerator extends Jd
    }

    Connection c = node.getDataSource().getConnection();
    +        PreparedStatement select = null;

    try {
    -            PreparedStatement select = c.prepareStatement(
    +            select = c.prepareStatement(
    SELECT_QUERY,
    ResultSet.TYPE_FORWARD_ONLY,
    ResultSet.CONCUR_UPDATABLE);
    @@ -91,6 +92,9 @@ public class DerbyPkGenerator extends Jd
    return nextId;
    }
    finally {
    +            if (select != null) {
    +                select.close();
    +            }
    c.close();
    }
    }
  • Andrus Adamchik at Feb 20, 2011 at 8:51 pm

    On Feb 20, 2011, at 10:38 PM, Andrey Razumovsky wrote:

    I'll reconsider changes in DerbyPkGenerator ASAP (no doubt for me that
    the changes were required though).
    Yeah the original code definitely required fixing. Besides I think this is not even a blocker for M2, but certainly something to revisit.

    Andrus

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupdev @
categoriescayenne
postedFeb 16, '11 at 12:23p
activeFeb 20, '11 at 8:51p
posts3
users2
websitecayenne.apache.org

People

Translate

site design / logo © 2022 Grokbase