Search code examples
javasql-serverc3p0jtds

Connection to SqlServer not closing in for loop java


I'm not sure the best practice for this, but my overall problem is that I can't figure out why my connection isn't closing.

I'm basically iterating through a list, and then inserting them into a table. Before I insert them into a table, I check and make sure it's not a duplicate. if it is, I update the row instead of inserting it. As of now, I can only get 13 iterations to work before the debug lets me know I had a connection not close.

Since I have 2 connections, I'm having trouble figuring out where I'm suppose to close my connections, and I was trying to use other examples to help. Here is what I got:

        Connection con = null;
    PreparedStatement stmt = null;
    PreparedStatement stmt2 = null;
    ResultSet rs = null;
    Connection con2 = null;


    for (Object itemId: aList.getItemIds()){
        try {
            con = cpds2.getConnection();
                stmt = con.prepareStatement("select [ID] from [DB].[dbo].[Table1] WHERE [ID] = ?");
                stmt.setInt(1, aList.getItem(itemId).getBean().getID());

                rs = stmt.executeQuery();
            //if the row is already there, update the data/

                if (rs.isBeforeFirst()){
                    System.out.println("Duplicate");
                    stmt2 = con2.prepareStatement("UPDATE [DB].[dbo].[Table1] SET "
                    + "[DateSelected]=GETDATE() where  [ID] = ?");
                    stmt2.setInt(1,aList.getItem(itemId).getBean().getID());
                stmt2.executeUpdate();
                }//end if inserting duplicate
                else{
                    con2 = cpds2.getConnection();
                    System.out.println("Insertion");
                    stmt.setInt(1, aList.getItem(itemId).getBean().getID());

                    //Otherwise, insert them as if they were new
                    stmt2 = con.prepareStatement("INSERT INTO [DB].[dbo].[Table1] ([ID],[FirstName],"
                            + "[LastName],[DateSelected]) VALUES (?,?,?,?)");
                    stmt2.setInt(1,aList.getItem(itemId).getBean().getID() );
                    stmt2.setString(2,aList.getItem(itemId).getBean().getFirstName());
                    stmt2.setString(3,aList.getItem(itemId).getBean().getLastName() );
                    stmt2.setTimestamp(4, new Timestamp(new Date().getTime()));
                    stmt2.executeUpdate();
                }//End Else
        }catch(Exception e){
                e.printStackTrace();
            }//End Catch
        finally{
                try { if (rs!=null) rs.close();} catch (Exception e) {}
            try { if (stmt2!=null) stmt2.close();} catch (Exception e) {}
            try { if (stmt!=null) stmt.close();} catch (Exception e) {}
            try { if (con2!=null) con2.close();} catch (Exception e) {}
            try {if (con!=null) con.close();} catch (Exception e) {}
            }//End Finally

    } //end for loop
    Notification.show("Save Complete");

This is my pooled connection:

    //Pooled connection
    cpds2 = new ComboPooledDataSource();

    try {
        cpds2.setDriverClass("net.sourceforge.jtds.jdbc.Driver");
    } catch (PropertyVetoException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    } //loads the jdbc driver
    cpds2.setJdbcUrl( "jdbc:jtds:sqlserver://SERVERNAME;instance=DB" );
    cpds2.setUser("username");
    cpds2.setPassword("password"); 
    cpds2.setMaxStatements( 180 ); 
    cpds2.setDebugUnreturnedConnectionStackTraces(true); //To help debug
    cpds2.setUnreturnedConnectionTimeout(2);  //to help debug

My main questions are, am i closing my connections right? Is my connection pool set up right? Should I be closing the connection inside the for loop or outside?

Is my problem with c3p0? Or JTDS?


Solution

  • It's great that you are working to be careful to robustly close() your resources, but this is overly complicated.

    Unless you are using a pretty old version of Java (something prior to Java 7) you can use try-with-resources, which really simplifies this stuff. Working with two different Connections in one logic unit-of-work invites misunderstandings. Resources should be a close()ed as locally to their use as possible, rather than deferring everything to the end.

    Your Exception handling is dangerous. If an Exception occurs that you don't understand, you might want to print its stack trace, but your code should signall the fact that whatever you were doing didn't work. You swallow the Exception, and even notify "Save Complete" despite it.

    All this said, your life might be made much easier by a MERGE statement, which I think SQL Server supports.

    Here is an (untested, uncompiled) example reorganization:

    try ( Connection con = cpds2.getConnection() ) {
        for (Object itemId: aList.getItemIds()){
            boolean id_is_present = false;
            try ( PreparedStatement stmt = con.prepareStatement("select [ID] from [DB].[dbo].[Table1] WHERE [ID] = ?") ) {
                stmt.setInt(1, aList.getItem(itemId).getBean().getID());
                try ( ResultSet rs = stmt.executeQuery() ) {
                   id_is_present = rs.next();
                }
            }
            if ( id_is_present ) {
                System.out.println("Duplicate");
                try ( PreparedStatement stmt = con.prepareStatement("UPDATE [DB].[dbo].[Table1] SET [DateSelected]=GETDATE() where  [ID] = ?") ) {
                    stmt.setInt(1,aList.getItem(itemId).getBean().getID());
                    stmt.executeUpdate();
                }
             } else {
                 System.out.println("Insertion");
                 try ( PreparedStatement stmt = con.prepareStatement("INSERT INTO [DB].[dbo].[Table1] ([ID],[FirstName], [LastName],[DateSelected]) VALUES (?,?,?,?)") ) {
                     stmt.setInt(1,aList.getItem(itemId).getBean().getID() );
                     stmt.setString(2,aList.getItem(itemId).getBean().getFirstName());
                     stmt.setString(3,aList.getItem(itemId).getBean().getLastName() );
                     stmt.setTimestamp(4, new Timestamp(new Date().getTime()));
                     stmt.executeUpdate();
                 }
             }
        }
        Notification.show("Save Complete");    
    }