-1

I have two queries running in parallel that fetch data from my remote db

public CompletableFuture<ObservableList<Appointment>> getAppointments(){
        return CompletableFuture.supplyAsync(() -> {
            return queryAppointments();
        });
    }

    public CompletableFuture<ObservableList<Customer>> getCustomers(){
        return CompletableFuture.supplyAsync(() -> {
           return queryCustomers();
        });
    }

This however causes a race condition because at the end of each query, I close the ResultSet and Statement so sometimes, not always, one query isn't finished and the ResultSet is closed before the second query is done and gives me an exception:

java.sql.SQLException: Operation not allowed after ResultSet closed

First query:

public ObservableList<Appointment> queryAppointments() {  
        ObservableList<Appointment> listAppointments = FXCollections.observableArrayList();
        Statement statement = null;
        ResultSet resultSet = null;

        try {
            statement = ConnectionManager.getConnection().createStatement();
            resultSet = statement.executeQuery(countriesQuery);
            while (resultSet.next()) {
                int appointmentId = resultSet.getInt("Appointment_ID");
                String title = resultSet.getString("Title");
                String description = resultSet.getString("Description");
                
                //Omitted Code for creating an appointment object here...

                listAppointments.add(appointment);
            }
            return listAppointments;
        } catch (SQLException e) {
            e.printStackTrace();
            return null;
        } finally {
            ConnectionManager.closeConnection();
            ConnectionManager.closeResultSetAndStatement(resultSet, statement);
        }
    }

Second Query:

public ObservableList<Customer> queryCustomers() {
        ObservableList<Customer> listCustomers = FXCollections.observableArrayList();
        Statement statement = null;
        ResultSet resultSet = null;

        try {
            statement = ConnectionManager.getConnection().createStatement();
            resultSet = statement.executeQuery(customersQuery);
            while (resultSet.next()) {
                int customerId = resultSet.getInt("Customer_Id");
                String name = resultSet.getString("Customer_Name");
                
                //Omitted code for creating customer object here

                listCustomers.add(customer);
            }

            return listCustomers;

        } catch (SQLException e) {
            e.printStackTrace();
            return null;
        } finally {
            ConnectionManager.closeConnection();
            ConnectionManager.closeResultSetAndStatement(resultSet, statement);
        }

    }
public class ConnectionManager {

    private static Connection connection;

    public static Connection getConnection() {
        try {
            connection = DriverManager.getConnection(Constants.CONNECTION_URL);
        } catch (SQLException e) {
            e.printStackTrace();
        }
        return connection;
    }

    public static void closeConnection(){
        try {
            connection.close();
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }

    public static void closeResultSetAndStatement(ResultSet resultSet, Statement statement){
        if (resultSet != null) {
            try {
                resultSet.close();
            } catch (SQLException sqlEx) { }
            resultSet = null;
        }

        if (statement != null) {
            try {
                statement.close();
            } catch (SQLException sqlEx) { }
            statement = null;
        }
    }

How should I deal with the race condition? I've tried thinking of a solution which would be performing one query after another instead of running it in parallel.

public static void main(String[] args) {
        mainRepository.getAppointments().thenAccept(retrievedAppointments -> {
            
             mainRepository.getCustomers().thenAccept(customers -> {
            
             });
        });

       
}

But couldn't the ResultSet also be cut off during the second query then? Is there a better solution in doing what I'm trying to accomplish?

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
DIRTY DAVE
  • 2,523
  • 2
  • 20
  • 83
  • 8
    Stop using global variables. Call `close` on the `Connection` you’re using, instead of calling `ConnectionManager.closeConnection();` to close an unspecified connection. I don’t see any reason for this `ConnectionManager` to exist. It doesn’t add any benefit to your code, just the race condition. When you start using [try-with-resources](https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html), you’ll have no need for those complicated closing routines. And well, you should rethink your exception handling… – Holger Aug 26 '21 at 06:47
  • 4
    You shouldn't use a single connection from concurrent threads. Either make sure you create a connection per thread, or ensure that the first query is complete before you start the second one. And I'm with Holger, your `ConnectionManager` seems like a bad idea (and is part of the problem here). – Mark Rotteveel Aug 26 '21 at 08:26
  • The `Statement` and `ResultSet` are local variables. *Ergo* it is impossible for another thread to close them prematurely. The problem here is that the `Connection` is *not* a local variable, and it needs to be. – user207421 Aug 26 '21 at 10:08
  • @user207421 It is possible to close the result set when the connection is in auto-commit: when another statement is executed in auto-commit, any previous result set from any statement of the connection should be closed, and given auto-commit is not explicitly disabled, it is enabled. – Mark Rotteveel Aug 26 '21 at 13:46

1 Answers1

-1

How about you use a flag? And close the ResultSet and Statement only when the flag is 0?

For example:

AtomicInteger flag = new AtomicInteger(0);

public ObservableList<Customer> queryCustomers() {
        flag.incrementAndGet(); // Flag is 2/1 now..
        ...
        // In your finally part
        int currFlag = flag.decrementAndGet();
        if(currFlag == 0){
            //close everything
        }
    }

public ObservableList<Appointment> queryAppointments() {  
        flag.incrementAndGet(); // Flag is 2/1 now..
        ...
        // In your finally part
        int currFlag = flag.decrementAndGet();
        if(currFlag == 0){
            //close everything
        }
    }

But still, the comment from @Holger seems to be more significant...

Renis1235
  • 4,116
  • 3
  • 15
  • 27