pgjdbc: Deadlock in PgStatement.killTimerTask
I’m submitting a …
- bug report
- feature request
Describe the issue I am facing a deadlock issue with my app server and with postgresql jdbc driver version 42.2.2. The issue traced down to the thread :
“Thread-134203” prio=5 Thread id=315839 TIMED_WAITING| java.lang.Object.wait(Native Method)| org.postgresql.jdbc.PgStatement.killTimerTask(PgStatement.java:1011)| org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:451)| org.postgresql.jdbc.PgStatement.execute(PgStatement.java:369)| org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:159)| org.postgresql.jdbc.PgPreparedStatement.executeQuery(PgPreparedStatement.java:109)|
From the release notes, this issue seems to be fixed in 42.2.0 version of the driver. But I had tested the above scenario with 42.2.12 version of the jar as well and the issue seems to persist.
Any help on resolving the issue?
Driver Version? 42.2.2
Java Version? 1.8.0_181
OS Version? Windows
PostgreSQL Version? 10.10
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 16 (10 by maintainers)
Commits related to this issue
- feat: synchronize statement executions This does not bring full thread safety to the driver, however, it should reduce the number of issues when connections and statements are accidentally executed c... — committed to vlsi/pgjdbc by vlsi 2 years ago
- feat: synchronize statement executions (e.g. avoid deadlock when Connection.isValid is executed from concurrent threads) This does not bring full thread safety to the driver, however, it should reduc... — committed to vlsi/pgjdbc by vlsi 2 years ago
- feat: synchronize statement executions (e.g. avoid deadlock when Connection.isValid is executed from concurrent threads) This does not bring full thread safety to the driver, however, it should reduc... — committed to vlsi/pgjdbc by vlsi 2 years ago
@vlsi , I just tried on latest version 42.4.0, but getting the same problem.
I have created a “silly” simple Java program to illustrate, you may please find it attached. This program starts 10 threads that share the same connection and just reads 10 records from any table and print the table id. On line 37, I am calling “connection.isValid(1);”
If you run this program and use jconsole, you will see that the threads are stuck. But if you comment out line 37 “connection.isValid(1);” then everything works fine.
_Name: Thread-9 State: TIMED_WAITING on org.postgresql.jdbc.PgConnection@bcdc3cb Total blocked: 1 028 Total waited: 2 860
Stack trace: java.base@11.0.11/java.lang.Object.wait(Native Method) app//org.postgresql.jdbc.PgStatement.killTimerTask(PgStatement.java:1043) app//org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:493) app//org.postgresql.jdbc.PgStatement.execute(PgStatement.java:408) app//org.postgresql.jdbc.PgPreparedStatement.executeWithFlags(PgPreparedStatement.java:181) app//org.postgresql.jdbc.PgPreparedStatement.executeUpdate(PgPreparedStatement.java:149) app//org.postgresql.jdbc.PgConnection.isValid(PgConnection.java:1436) app//PostgresTest$TestClass.run(PostgresTest.java:37) java.base@11.0.11/java.lang.Thread.run(Thread.java:829)_
PostgresTest.zip .
@omardawod119 , I’ve prepared a PR: https://github.com/pgjdbc/pgjdbc/pull/2575 I wonder if you could test it.
That is https://github.com/pgjdbc/pgjdbc/issues/1951
I don’t think synchronization at that point changes the semantics so +1
Ok, now I see what happens:
PgConnectionreuses statement forisValidcheck: https://github.com/pgjdbc/pgjdbc/blob/6e453aaead9dc15bf648cb490e6bbef6a1a2c3a7/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java#L1457-L1459PgStatementinitializes/cleans up timer outside ofsynchronizedblock: https://github.com/pgjdbc/pgjdbc/blob/6e453aaead9dc15bf648cb490e6bbef6a1a2c3a7/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java#L485-L493I’m inclined to add synchronization over https://github.com/pgjdbc/pgjdbc/blob/6e453aaead9dc15bf648cb490e6bbef6a1a2c3a7/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java#L404-L419
We seem already have similar synchronized blocks anyway, and it does solve the testcase provided by @omardawod119