Subject: [pgjdbc/pgjdbc] issue #3462: Next Method of PgResultSet Throws NullPointerException Instead of SQLException

From: "butopcu (@butopcu)" <[email-masked]>
Date: Tue, 17 Dec 2024 14:06:00 +0000

### **Describe the Issue**
We are using PostgreSQL with the JDBC driver in our project, and some of our code utilizes multi-threading/task logic to fetch data from the database. In our scenario, we have 4 threads and 8 database connections. The task logic splits the query IDs into 4 parts, and each thread executes a main query followed by additional queries for each entry to populate data objects.

At runtime, 8 `ResultSet` objects remain open because of this, and occasionally, we encounter a `PSQLException` with the message "This ResultSet is closed," which we handle by saving the failed IDs and retrying at the end of the task. 

However, in a new scenario, the method unexpectedly throws a `NullPointerException`, while we typically only catch `SQLExceptions` in our code. This makes it more difficult to handle the exception.

### **Driver Version?**
42.x

### **Java Version?**
OpenJDK 8

### **OS Version?**
Red Hat Enterprise Linux 8.10

### **PostgreSQL Version?**
PostgreSQL 11.x

### **To Reproduce**
1. Open `ResultSet` with Connection 1 (Bulk Query to Table A).
2. For each entry in the `ResultSet`, execute another query with Connection 2 (Single Query to Table B).
3. Populate data objects by setting fields based on the results from Table B.
4. Close `ResultSet` 2 after processing.
5. After completing the loop, close `ResultSet` 1.

### **Expected Behavior**
We expect to catch `SQLExceptions` resulting from database operations, not `NullPointerExceptions`.

### **Logs**
Trace:
```
java.lang.NullPointerException
    at org.postgresql.jdbc.PgResultSet.next(PgResultSet.java:2119)
```

Other Mentioned Exceptions

```
  org.postgresql.util.PSQLException: This ResultSet is closed.
  	at org.postgresql.jdbc.PgResultSet.checkClosed(PgResultSet.java:3096)
  	at org.postgresql.jdbc.PgResultSet.findColumn(PgResultSet.java:2934)
  	at org.postgresql.jdbc.PgResultSet.getString(PgResultSet.java:2796)
```

### **Draft Code Example (Assume It's Run Over Multiple Threads)**

```java
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.HashMap;
import java.util.Map;

public class SeparateQueriesExample {
    public static void main(String[] args) {
        String url = "jdbc:postgresql://localhost:5432/mydatabase"; 
        String user = "username";
        String password = "password";

        int[] ids = {1, 2, 3, 4};

        StringBuilder queryTableA = new StringBuilder("SELECT id, name FROM tablea WHERE id IN (");
        for (int i = 0; i < ids.length; i++) {
            queryTableA.append("?");
            if (i < ids.length - 1) {
                queryTableA.append(", ");
            }
        }
        queryTableA.append(")");

        String queryTableB = "SELECT location FROM tableb WHERE name = ?";

        try (Connection conn = DriverManager.getConnection(url, user, password);
             PreparedStatement pstmtTableA = conn.prepareStatement(queryTableA.toString());
             PreparedStatement pstmtTableB = conn.prepareStatement(queryTableB)) {

            for (int i = 0; i < ids.length; i++) {
                pstmtTableA.setInt(i + 1, ids[i]);
            }

            ResultSet rsTableA = pstmtTableA.executeQuery();
            Map<Integer, String> idNameMap = new HashMap<>();

            while (rsTableA.next()) {
                int id = rsTableA.getInt("id");
                String name = rsTableA.getString("name");
                idNameMap.put(id, name);
            }

            for (Map.Entry<Integer, String> entry : idNameMap.entrySet()) {
                int id = entry.getKey();
                String name = entry.getValue();

                pstmtTableB.setString(1, name);

                try (ResultSet rsTableB = pstmtTableB.executeQuery()) {
                    while (rsTableB.next()) {
                        String location = rsTableB.getString("location");
                    }
                }
            }

        } catch (SQLException e) {
            e.printStackTrace();
        }
    }
}
```

---

### **Summary of Issue**
- The application uses multiple threads to query data from the database, causing several `ResultSet` objects to remain open during execution.
- Occasionally, a `NullPointerException` is thrown unexpectedly, though we are only expecting to handle `SQLExceptions`.
- We are unsure why the `NullPointerException` occurs in this multi-threaded context, especially since we handle SQL exceptions with `try-catch` blocks.
---

### **My Analysis:**
The `ResultSet` is unexpectedly closing, and the `rows` variable is being set to `null`. This occurs after the `checkClosed()` method is called, which causes a `NullPointerException` instead of the expected `SQLException`.

---

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 17 Dec 2024 14:42:17 +0000

@butopcu, could you provide more stack trace lines for the NPE?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 17 Dec 2024 14:49:15 +0000

What is the exact pgjdbc version that you use?

~It looks like `PgResultSet.next(PgResultSet.java:2119)` relates to either `ResultSet#updateRow()` or `ResultSet#insertRow()`. Could you review `updateRow` / `insertRow` usages?~ <- This assumed the latest version.

---

From: "butopcu (@butopcu)" <[email-masked]>
Date: Tue, 17 Dec 2024 15:18:49 +0000

The exact version seems to be: 42.5.1.

![image](https://github.com/user-attachments/assets/f7ec0ffe-f30e-4d56-b6fd-e26747ce5d99)

The rest of the stack traces are from our private classes, which I haven't mentioned. Our ResultSet implementation simply calls next().

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Wed, 18 Dec 2024 07:35:22 +0000

@butopcu , the NPE at 2119 most likely means you have two threads: one that accesses `.next()` and another one that closes the resultset (or calls `next` as well). The thing is `rows` field was **not null** at both line 2019 and 2116, and then it became null at line `2019`. It definitely means there was a concurrent thread that nullified the field.

I do not think we could do much about it, and I would suggest you try figuring out if the same `ResultSet` or `Statement` is reused across different threads (e.g. `static ResultSet` or `static Statement`).

---

From: "butopcu (@butopcu)" <[email-masked]>
Date: Wed, 18 Dec 2024 19:25:15 +0000

@vlsi 
Actually, each thread uses new `ResultSet` and `Statement` objects as these are local variables.  
Our Database Team has thoroughly investigated the code and PostgreSQL logs but couldn't determine why the `ResultSet` occasionally and rarely switches to a closed state.

The same SQL queries are being executed with different parameters across multiple threads:

**Thread 1**:  
- Integers: `WHERE IN (1..500)` (Main SQL Query to TableA)  
  - The `ResultSet` remains open (`TYPE_SCROLL_INSENSITIVE`).  
- Process all data in a loop:  
  - `WHERE = ID` (Subquery to TableB for each entry returned from TableA). (New ResultSet, Connection and Statement)

**Thread 2**:  
- Integers: `WHERE IN (501..1000)` (Main SQL Query to TableA).  
- Process all data in a loop:  
  - `WHERE = ID` (Subquery to TableB for each entry returned from TableA).

The only static object is the SQL query string, such as:

`private static final String QUERY_BY_ID= "SELECT name, adress FROM TABLEB WHERE ID=?";`

---

From: "butopcu (@butopcu)" <[email-masked]>
Date: Fri, 20 Dec 2024 01:05:08 +0000

@vlsi 

Hi;

I have a quick question for you: Is it a good idea to assign the size of `rows` to a variable and avoid calling `.size()` repeatedly, like this:

```java
final int rowsSize = rows.size();
```

[Commit](https://github.com/butopcu/pgjdbc/commit/67b0994449665aabd5d843f66c62f9a65ea38015)

If it doesn't break anything, I suppose the size won't change while we're processing the `ResultSet`. 
Could this approach help in my case?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Fri, 20 Dec 2024 07:19:22 +0000

1) This change alone won't make the code thread-safe, so extracting a local variable won't resolve the concurrency issue
2) `this.rows` might be re-assigned after `.fetch`, see https://github.com/butopcu/pgjdbc/blob/67b0994449665aabd5d843f66c62f9a65ea38015/pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java#L985-L987, so it might be tempting to reuse the local variable, yet it would not be the right thing to do

By the way, could you try running the application with `-ea` Java command line option to enable assertions? It might reveal `null` inputs to `castNonNull`.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Fri, 20 Dec 2024 10:20:47 +0000

`Actually, each thread uses new ResultSet and Statement objects as these are local variables.`

Are you sharing connections across threads ?

---

From: "butopcu (@butopcu)" <[email-masked]>
Date: Fri, 20 Dec 2024 19:54:33 +0000

@davecramer

Yes, our connection pool mechanism is not closing and reopening connections.  
When a Thread takes a connection, it is removed from the free connection list.  
After the Thread completes its task, we close the `ResultSet` and `Statement`, then push the connection back to the free connection list. After that, any other thread can take and reuse the same connection.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Fri, 20 Dec 2024 21:09:57 +0000

I'd probably look at that to make sure they are really closed before being used again.

Try with resources would probably be useful here.

---

From: "davecramer (@davecramer)" <[email-masked]>
Date: Thu, 26 Dec 2024 22:05:20 +0000

can we close this ?

---

From: "svendiedrichsen (@svendiedrichsen)" <[email-masked]>
Date: Thu, 26 Dec 2024 23:29:58 +0000

Why create your own connection pool and not use an already existing one?

---

From: "butopcu (@butopcu)" <[email-masked]>
Date: Fri, 27 Dec 2024 01:03:56 +0000

I'm closing as not planned to fix; probably causing by our own implementation.

Thanks for suggestions.