Is this use of PreparedStatements in a Thread in Java correct?

Question

I'm still an undergrad just working part time and so I'm always trying to be aware of better ways to do things. Recently I had to write a program for work where the main thread of the program would spawn "task" threads (for each db "task" record) which would perform some operations and then update the record to say that it has finished. Therefore I needed a database connection object and PreparedStatement objects in or available to the ThreadedTask objects.

This is roughly what I ended up writing, is creating a PreparedStatement object per thread a waste? I thought static PreparedStatments could create race conditions...

Thread A stmt.setInt();
Thread B stmt.setInt();
Thread A stmt.execute();  
Thread B stmt.execute();  

A´s version never gets execed..

Is this thread safe? Is creating and destroying PreparedStatement objects that are always the same not a huge waste?

public class ThreadedTask implements runnable {
    private final PreparedStatement taskCompleteStmt;

    public ThreadedTask() {
        //...
        taskCompleteStmt = Main.db.prepareStatement(...);
    }

    public run() {
        //...
        taskCompleteStmt.executeUpdate();
    }
}

public class Main {
    public static final db = DriverManager.getConnection(...);
}

Answer 1

I believe it is not a good idea to share database connections (and prepared statements) between threads. JDBC does not require connections to be thread-safe, and I would expect most drivers to not be.

Give every thread its own connection (or synchronize on the connection for every query, but that probably defeats the purpose of having multiple threads).

Is creating and destroying PreparedStatement objects that are always the same not a huge waste?

Not really. Most of the work happens on the server, and will be cached and re-used there if you use the same SQL statement. Some JDBC drivers also support statement caching, so that even the client-side statement handle can be re-used.

You could see substantial improvement by using batched queries instead of (or in addition to) multiple threads, though. Prepare the query once, and run it for a lot of data in a single big batch.

Answer 2

You're best to use a connection pool and get each thread to request a connection from the pool. Create your statements on the connection you're handed, remembering to close it and so release it back to the pool when you're done. The benefit of using the pool is that you can easily increase the number of available connections should you find that thread concurrency is becoming an issue.

Answer 3

The threadsafety is not the issue here. All looks syntactically and functionally fine and it should work for about half a hour. Leaking of resources is however the real issue here. The application will crash after about half a hour because you never close them after use. The database will in turn sooner or later close the connection itself so that it can claim it back.

That said, you don't need to worry about caching of preparedstatements. The JDBC driver and the DB will take care about this task. Rather worry about resource leaking and make your JDBC code as solid as possible.

public class ThreadedTask implements runnable {
    public run() {
        Connection connection = null;
        Statement statement = null;
        try {
            connection = DriverManager.getConnection(url);
            statement = connection.prepareStatement(sql);
            // ...
        } catch (SQLException e) {
            // Handle?
        } finally {
            if (statement != null) try { statement.close(); } catch (SQLException logOrIgnore) {}
            if (connection != null) try { connection.close(); } catch (SQLException logOrIgnore) {}
        }
    }
}

To improve connecting performance, make use of a connection pool like c3p0 (this by the way does not mean that you can change the way how you write the JDBC code; always acquire and close the resources in the shortest possible scope in a try-finally block).