62

I have a problem with try-with-resources and I am asking just to be sure. Can I use it, if I need to react on exception, and I still need the resource in catch block? Example given is this:

try (java.sql.Connection con = createConnection())
{
    con.setAutoCommit(false);
    Statement stm = con.createStatement();
    stm.execute(someQuery); // causes SQLException
}
catch(SQLException ex)
{
    con.rollback();
    // do other stuff
}

I fear that I am still doomed to use the old try-catch-finally in this case, even according to oracle documentation - "catch and finally blocks in a try-with-resources statement, any catch or finally block is run after the resources declared have been closed."

Jan Hruby
  • 1,477
  • 1
  • 13
  • 26
  • 2
    In this case if connection itself has failed, there is no point of rolling it back. The scope of `con` is limited to try block only. – bprasanna Apr 02 '13 at 11:55
  • This question may help as well. http://stackoverflow.com/questions/9260159/java-7-automatic-resource-management-jdbc – Kevin S Oct 23 '13 at 19:42
  • Of all the interesting options given, I still prefer the original `try-catch-finally` – Adam Oct 12 '17 at 14:27

4 Answers4

64

According to the language spec, the connection will be closed before the catch clause is executed (http://docs.oracle.com/javase/specs/jls/se7/html/jls-14.html#jls-14.20.3.2).

A possible solution is to nest try-with-resources statements:

try (java.sql.Connection con = createConnection())
{
    con.setAutoCommit(false);
    try (Statement stm = con.createStatement())
    {
        stm.execute(someQuery); // causes SQLException
    }
    catch(SQLException ex)
    {
        con.rollback();
        con.setAutoCommit(true);
        throw ex;
    }
    con.commit();
    con.setAutoCommit(true);
}

Hopefully that illustrates the point. This should be improved quite a bit if you plan on using it in production code.

For instance, if you are using a connection pool, then you must return the connection as you got it, so con.setAutoCommit(true); should be done in a finally clause. This would mean that the outer try-with-resources should be a traditional try-catch-finally.

Edit (2018)

I see people commenting on this still, so I thought I'd give it a 2018 reply. I am not working in Java anymore, mainly been working in Scala, Clojure and Kotlin, and this code has not been tested, so please treat this as just another example. However since Java has lambdas, I think the following approach is a lot better. And I've done similar things in production code in these other languages.

In this approach there is a inTransaction function handling all the nasty transaction stuff. But usage is pretty simple.

public class Foo {

    interface ConnectionProvider {
        Connection get() throws SQLException;
    }

    public static <A> A doInTransation(ConnectionProvider connectionProvider, Function<Connection, A> f) throws SQLException {
        Connection connection = null;
        A returnValue;
        boolean initialAutocommit = false;
        try {
            connection = connectionProvider.get();
            initialAutocommit = connection.getAutoCommit();
            connection.setAutoCommit(false);
            returnValue = f.apply(connection);
            connection.commit();
            return returnValue;
        } catch (Throwable throwable) {
            // You may not want to handle all throwables, but you should with most, e.g.
            // Scala has examples: https://github.com/scala/scala/blob/v2.9.3/src/library/scala/util/control/NonFatal.scala#L1
            if (connection != null) {
                connection.rollback();
            }
            throw throwable;
        } finally {
            if (connection != null) {
                try {
                    if(initialAutocommit){
                        connection.setAutoCommit(true);
                    }
                    connection.close();
                } catch (Throwable e) {
                    // Use your own logger here. And again, maybe not catch throwable,
                    // but then again, you should never throw from a finally ;)
                    StringWriter out = new StringWriter();
                    e.printStackTrace(new PrintWriter(out));
                    System.err.println("Could not close connection " + out.toString());
                }
            }
        }
    }

    public static void main(String[] args) throws SQLException {
        DataSource ds = null;

        // Usage example:
        doInTransation(ds::getConnection, (Connection c) -> {
            // Do whatever you want in a transaction
            return 1;
        });
    }
}

I would hope there is some battle tested libraries out there doing this stuff for you, there is at least in these other languages.

I see there are several comments regarding autocommit and connection pools. The above examples should be agnostic to where the connection came from, a pool or not, i.e. only setting it back to true if that was it's initial value. So if from a pool it is false, it should not be touched.

A final word on try-with-resources. I don't think it is a very good abstraction, so I'd be careful using it in more complex scenarios.

Alf
  • 799
  • 6
  • 5
  • 13
    Be aware that calling [`setAutoCommit`](http://docs.oracle.com/javase/8/docs/api/java/sql/Connection.html#setAutoCommit-boolean-) calls a COMMIT on any pending transaction. So be careful in more complicated code. – Basil Bourque Aug 13 '15 at 22:43
  • 1
    "This would mean that the outer try-with-resources should be a traditional try-catch-finally." - so in fact the "try with resources" pattern is completely useless for JDBC connections with connections pools...? – Itai Mar 07 '16 at 19:05
  • 1
    "For instance, if you are using a connection pool, then you must return the connection as you got it, so con.setAutoCommit(true); should be done in a finally clause." — If the connection pool adheres to the JDBC specification, that's not true. According to the spec, a newly checked out connection must always have autocommit enabled. Most connection pools like c3p0 will ensure this even if the connection was checked in with autocommit disabled. – Jackson Apr 10 '17 at 17:41
  • @Alf in the last line, why do you need to set the autocommit to "true" if you've just commited and the connection is being closed immediately after that? – Luigi Cortese Jun 09 '17 at 09:24
  • 3
    But you only catch `SQLException`. What if a non-checked exception, such as `NullPointerException` occurs? Will your transaction never be rolled back? – Garret Wilson May 21 '18 at 17:12
  • @Alf: this is a good solution but sonar throws the error stating nested try blocks not allowed. How to achieve both in your example? – Gaurav Nov 21 '18 at 12:39
25

In your code you are catching "SQLException" to perform the autoCommit reset. Any kind of runtime exception (like a null pointer exception) will bubble from your code without resetting the auto-commit.

The try-with-resource syntax causes the compiler to generate some wonderful code to cover all execution paths and to keep up with all suppressed exceptions through the closings. With a couple of helper classes you can insert commit/rollback and reset-auto-commit into the code generation process:

import java.sql.SQLException;
import java.sql.Connection;

public class AutoRollback implements AutoCloseable {

    private Connection conn;
    private boolean committed;

    public AutoRollback(Connection conn) throws SQLException {
        this.conn = conn;        
    }

    public void commit() throws SQLException {
        conn.commit();
        committed = true;
    }

    @Override
    public void close() throws SQLException {
        if(!committed) {
            conn.rollback();
        }
    }

}

public class AutoSetAutoCommit implements AutoCloseable {

    private Connection conn;
    private boolean originalAutoCommit;

    public AutoSetAutoCommit(Connection conn, boolean autoCommit) throws SQLException {
        this.conn = conn;
        originalAutoCommit = conn.getAutoCommit();
        conn.setAutoCommit(autoCommit);
    }

    @Override
    public void close() throws SQLException {
        conn.setAutoCommit(originalAutoCommit);
    }

}

Now you can control rollback and autocommit with the "try with resource" syntax like this:

    try(Connection conn = getConnection(),
        AutoSetAutoCommit a = new AutoSetAutoCommit(conn,false),
        AutoRollback tm = new AutoRollback(conn)) 
    {

        // Do stuff

        tm.commit();
    } 
ChrisCantrell
  • 3,833
  • 1
  • 22
  • 14
  • 2
    Interesting code, I didn't take the time to try AutoCloseable interface. This seems to be a good approche to replace the finally clause. In production the code would be much simpler with only one try-with-resource. Nice ! – AxelH May 18 '16 at 10:58
  • 1. Why don't you declare "throws SQLException" in close() methods? Why do you wrap as just an Exception? 2. Why don't you create all resources within a single try block? AFAIK they would be closed in an opposite order, you've declared them. – Sabine Jan 28 '17 at 10:48
  • 1
    Very good points! I'll change the closes to throw SQL Exceptions and combine them in one try as you and AxelH suggested. Thanks for the feedback. – ChrisCantrell Jan 30 '17 at 14:06
  • @ChrisCantrell: this is excellent. works flawlessly. fixes sonar recursive try-block error as well! – Gaurav Nov 21 '18 at 14:30
  • I prefer this to the lambda solution, but why 2 classes and not just all the functionality in one? You can drop one line from the `try` parentheses. The only difficulty I see in combining them is what to name it. – Adam Dec 10 '18 at 15:46
  • The AutoSetAutoCommit is an optional feature. You can leave it out if you don't need it. I added it to my answer because the OP was using auto-commit. It needs to be a separate "closeable" because the "setAutoCommit" can throw an Exception. Thus all three resources (connection, autoCommitter, and autoRollback) resolve no matter what happens in the others. And all of the exceptions from the resolutions are chained together without any getting lost. – ChrisCantrell Dec 11 '18 at 13:15
  • about error handling, I was using EJB3, I've used interceptors over the bean for custom unchecked exceptions. Maybe I got too fixated on the example you gave, but if you have to handle standard unchecked exceptions like NPE in your code, I'd say that problem is not insufficient exception handling – Jan Hruby Feb 24 '19 at 22:46
  • @jan I'm not sure I understand your comment. The original question is about intercepting exceptions of any kind (checked or unchecked) and finalizing resources with try-with-resource before the exceptions bubble up the stack. The actual handling of said exception happens above/around this code and isn't part of the discussion here. – ChrisCantrell Feb 26 '19 at 16:09
  • 1
    @Chris - sorry, I have misinterpreted your answer, I thought your focus was on missing exception handling in my example but I see now it was on autocommit. Anyway, original question had nothing to do with exception handling but was simply about scope of the resources defined in the header of try block – Jan Hruby Feb 26 '19 at 16:40
  • 3
    Need to replace the ","s in the try-resource with ";". At least that's what mine wants. – cs94njw Mar 11 '19 at 15:43
11
    //try with resources
    try(Connection conn = this.connectionProvider.getConnection()){//auto close BEFORE reach this , catch block, so we need a inner try block for statement
        boolean oldAutoCommit=conn.getAutoCommit();
        conn.setAutoCommit(false);//auto commit to false
        try(
            Statement stm = con.createStatement()
        ){
            stm.execute(someQuery); // causes SQLException
            conn.commit();//commit
        }
        catch (SQLException ex){
            conn.rollback();//error, rollback
            throw ex;//If you need to throw the exception to the caller
        }
        finally {
            conn.setAutoCommit(oldAutoCommit);//reset auto commit
        }
    }
JaskeyLam
  • 15,405
  • 21
  • 114
  • 149
5

In the example above I think it's better to put con.commit() inside nested try-catch because it also can throw SQLException.

 try (java.sql.Connection con = createConnection())
    {
        con.setAutoCommit(false);
        try (Statement stm = con.createStatement())
        {
            stm.execute(someQuery); // causes SQLException
            con.commit();           // also causes SQLException!
        }
        catch(SQLException ex)
        {
            con.rollback();
            throw ex;
        }finally{
            con.setAutoCommit(true);
        }
    }

We had such problem in our production environment with unclosed sessions.

Ashish Kumar
  • 916
  • 2
  • 15
  • 32
shambalaxx
  • 51
  • 3
  • 2
  • 4
    You should use a finally statement to set the autocommit back to true. This would prevent you to have this statement twice. – AxelH May 21 '16 at 17:39