First of all, I don’t have much experience with thread safe programming.
I have a MySQL class, and I want to use one instance in multiple threads to prevent blocking code in the main thread. I read about connection pooling but I want to keep it as simple as it is.
This is my MySQL class:
package com.vanillage.bukkitutils.mysql;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.SQLException;
public class MySQL {
private final String host;
private final int port;
private final String database;
private final String user;
private final String password;
private Connection connection;
public MySQL(String host, int port, String database, String user, String password) {
if (host == null) {
//TODO
}
if (database == null) {
//TODO
}
if (user == null) {
//TODO
}
if (password == null) {
//TODO
}
this.host = host;
this.port = port;
this.database = database;
this.user = user;
this.password = password;
}
public String getHost() {
return host;
}
public int getPort() {
return port;
}
public String getDatabase() {
return database;
}
public String getUser() {
return user;
}
public String getPassword() {
return password;
}
public Connection getConnection() {
return connection;
}
public synchronized void connect() throws SQLException {
connection = DriverManager.getConnection("jdbc:mysql://" + host + ":" + port + "/" + database + "?autoReconnect=true", user, password);
}
public synchronized void checkConnection(int timeout) throws SQLException {
if (connection == null) {
connect();
} else {
boolean connectionValid = false;
try {
connectionValid = connection.isValid(timeout);
} catch (SQLException e) {
e.printStackTrace();
connect();
connectionValid = true;
}
if (!connectionValid) {
connect();
}
}
}
public synchronized ResultSet query(String query) throws SQLException {
return connection.prepareStatement(query).executeQuery();
}
public synchronized boolean update(String query) throws SQLException {
return connection.prepareStatement(query).execute();
}
public synchronized void close() throws SQLException {
if (connection != null) {
connection.close();
connection = null;
}
}
public synchronized boolean hasConnection(boolean checkOpen, boolean checkValid, int timeout) throws SQLException {
return connection != null && (!checkOpen || !connection.isClosed()) && (!checkValid || connection.isValid(timeout));
}
@Override
public String toString() {
return host + ":" + port + ", " + database + ", " + user + ", " + password;
}
}
Is it possible to make my MySQL class Thread safe with the synchronized keyword, as I already used it in the code above?
I use this class like that from different threads:
try {
mySQL.checkConnection(0);
try {
ResultSet resultSet = mySQL.query("SELECT * FROM Example");
if (resultSet.next()) {
System.out.println(resultSet.getString("Example"));
}
} catch (SQLException e) {
System.out.println("Error while executing query: " + e.getMessage());
}
} catch (SQLException e) {
System.out.println("Could not create a valid connection: " + e.getMessage());
}
My question: Is it thread safe?
Edit:
package testprogramm;
import java.beans.PropertyVetoException;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.Map;
import java.util.Scanner;
import com.mchange.v2.c3p0.ComboPooledDataSource;
public class TestProgramm {
private static final Scanner scanner = new Scanner(System.in);
private static Map<String, ComboPooledDataSource> dataSources = new HashMap<>();
public static void main(String[] args) {
//setup connections
ComboPooledDataSource testDataSource = new ComboPooledDataSource();
try {
testDataSource.setDriverClass("com.mysql.jdbc.Driver");
} catch (PropertyVetoException e) {
e.printStackTrace();
}
testDataSource.setJdbcUrl("jdbc:mysql://localhost:3306/database");
testDataSource.setUser("user");
testDataSource.setPassword("password");
dataSources.put("test", testDataSource);
while (true) {
String line = scanner.nextLine();
ComboPooledDataSource dataSource = dataSources.get(line);
if (dataSource != null) {
new Thread(new Runnable() {
ComboPooledDataSource dataSource = null;
public Runnable init(ComboPooledDataSource dataSource) {
this.dataSource = dataSource;
return this;
}
@Override
public void run() {
try {
Connection connection = dataSource.getConnection();
PreparedStatement preparedStatement = connection.prepareStatement("SELECT * FROM Example");
ResultSet resultSet = preparedStatement.executeQuery();
int i = 0;
while (resultSet.next()) {
i ++;
}
resultSet.close();//TODO: move to finally clause with null check and try/catch
preparedStatement.close();
connection.close();
System.out.println(i + " entries");
} catch (SQLException e) {
System.out.println("Error while executing statement: " + e.getMessage());
}
}
}.init(dataSource)).start();
} else {
System.out.println("No such connection");
}
}
}
}
Advertisement
Answer
Your question: Is it thread safe?
My answer: No, it’s not.
Most simple way to break it: Have one of your threads call mySQL.getConnection().close();
Apart from that: Most connections don’t like parallel statements at all. What should be the transaction scope of that?
You should seriously consider the use of a connection pool instead. My favorite choisce would be c3p0. See http://www.mchange.com/projects/c3p0/#quickstart for a quickstart example.
Making it safe
Instead of passing around an instance of MySQL, you create and configure a ComboPooledDataSource (or any other DataSource you want to use). Then inside your classes, obtain a Connection from that pool, perform your SQL statements and then close it. The most convenient way to do that would be with try-with-resource introduced with Java 7:
try(Connection con = pool.getConnection();
PreparedStatement ps = con.prepareStatement("SELECT * FROM whatever");
ResultSet rs = ps.executeQuery()) {
while(rs.next()) {
//handle resultset
}
}
Some more info on your existing class
You fail to clean up a whole lot of Statements if you do
public synchronized ResultSet query(String query) throws SQLException {
//Statement never closed
return connection.prepareStatement(query).executeQuery();
}
public synchronized boolean update(String query) throws SQLException {
//Statement never closed
return connection.prepareStatement(query).execute();
}