Thread: Deadlock while using getNotifications() and Statement.executeQuery()
Hello! I'm using more than one thread in my application. In one thread I listen to notifications from the database and another onechecks if the connection is alive by doing a "select 1". Sometimes I get a deadlock!!! I'm using postgresql-8.2-508.jdbc4.jar. The thread dump gives me this: Java stack information for the threads listed above: =================================================== "DB connection select 1": at org.postgresql.core.v3.ProtocolConnectionImpl.setTransactionState(ProtocolConnectionImpl.java:191) - waiting to lock <0xb1a00f88> (a org.postgresql.core.v3.ProtocolConnectionImpl) at org.postgresql.core.v3.QueryExecutorImpl.receiveRFQ(QueryExecutorImpl.java:1654) at org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:1410) at org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:193) - locked <0xb1a033e0> (a org.postgresql.core.v3.QueryExecutorImpl) at org.postgresql.jdbc2.AbstractJdbc2Statement.execute(AbstractJdbc2Statement.java:452) at org.postgresql.jdbc2.AbstractJdbc2Statement.executeWithFlags(AbstractJdbc2Statement.java:337) at org.postgresql.jdbc2.AbstractJdbc2Statement.executeQuery(AbstractJdbc2Statement.java:236) at database.DatabaseManager$SelectRunnable.run(DatabaseManager.java:483) at java.lang.Thread.run(Thread.java:619) "Database tables listener": at org.postgresql.core.v3.QueryExecutorImpl.processNotifies(QueryExecutorImpl.java:547) - waiting to lock <0xb1a033e0> (a org.postgresql.core.v3.QueryExecutorImpl) at org.postgresql.core.v3.ProtocolConnectionImpl.getNotifications(ProtocolConnectionImpl.java:68) - locked <0xb1a00f88> (a org.postgresql.core.v3.ProtocolConnectionImpl) at org.postgresql.jdbc2.AbstractJdbc2Connection.getNotifications(AbstractJdbc2Connection.java:1013) at database.DbListenerHandler$DbListener.run(DbListenerHandler.java:134) at java.lang.Thread.run(Thread.java:619) Found 1 deadlock. I've marked the places where the deadlock occurs with "\\<<<<<<<<<<<". In the "DB connection select 1" thread I have: ==================================== success = false; Statement stmt = null; try { int i = 0; stmt = getCon().createStatement(); ResultSet rs = stmt.executeQuery("SELECT 1");\\<<<<<<<<<<< if (rs.next()) { i = rs.getInt(1); } if (i == 1) { success = true; } } catch (SQLException ex) { } finally { if (stmt != null) { try { stmt.close(); } catch (SQLException ex) { } } } ==================================== And in the "Database tables listener" thread I have: ==================================== while (...) { try { PGNotification notifications[] =pgconn.getNotifications();\\<<<<<<<<<<< if (notifications != null) { .... } Thread.sleep(300); } catch (SQLException sqle) { .... } catch (InterruptedException ie) { .... } } ==================================== Is this a bug in jdbc-postgres? Joao Leal
I did a workaround/hack to fix the problem, but there should be some better way to fix this. I had to make sure that the locking order in getNotifications() is same as in executeQuery(), but that meant exposing the QueryExecutor in the ProtocolConnection (the QueryExecutorImpl is saved as private variable inside ProtocolConnectionImpl). So I had to make the following changes (it's not pretty, I know...): ===================================== org/postgresql/jdbc2/AbstractJdbc2Connection.java 1012,1014c1012,1016 < // Backwards-compatibility hand-holding. < PGNotification[] notifications = protoConnection.getNotifications(); < return (notifications.length == 0 ? null : notifications); --- > synchronized(protoConnection.getQueryExecutorImpl()) { > // Backwards-compatibility hand-holding. > PGNotification[] notifications = protoConnection.getNotifications(); > return (notifications.length == 0 ? null : notifications); > } ===================================== org/postgresql/core/v2/ProtocolConnectionImpl.java and org/postgresql/core/v3/ProtocolConnectionImpl.java > public QueryExecutorImpl getQueryExecutorImpl() { > return executor; > } > ===================================== org/postgresql/core/ProtocolConnection.java 13a14 > import org.postgresql.core.QueryExecutor; 134a136,141 > > /** > * Used only to lock the executor object > * @return > */ > public QueryExecutor getQueryExecutorImpl(); With these changes the problem was solved, but not in the best way. Any comments? On Monday 24 March 2008, Joao Rui Leal wrote: > Hello! > > I'm using more than one thread in my application. In one thread I listen to > notifications from the database and another one checks if the connection is > alive by doing a "select 1". Sometimes I get a deadlock!!! > I'm using postgresql-8.2-508.jdbc4.jar. > > The thread dump gives me this: > > Java stack information for the threads listed above: > =================================================== > "DB connection select 1": > at > org.postgresql.core.v3.ProtocolConnectionImpl.setTransactionState(ProtocolC >onnectionImpl.java:191) - waiting to lock <0xb1a00f88> (a > org.postgresql.core.v3.ProtocolConnectionImpl) at > org.postgresql.core.v3.QueryExecutorImpl.receiveRFQ(QueryExecutorImpl.java: >1654) at > org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.j >ava:1410) at > org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:193 >) - locked <0xb1a033e0> (a org.postgresql.core.v3.QueryExecutorImpl) at > org.postgresql.jdbc2.AbstractJdbc2Statement.execute(AbstractJdbc2Statement. >java:452) at > org.postgresql.jdbc2.AbstractJdbc2Statement.executeWithFlags(AbstractJdbc2S >tatement.java:337) at > org.postgresql.jdbc2.AbstractJdbc2Statement.executeQuery(AbstractJdbc2State >ment.java:236) at > database.DatabaseManager$SelectRunnable.run(DatabaseManager.java:483) at > java.lang.Thread.run(Thread.java:619) > "Database tables listener": > at > org.postgresql.core.v3.QueryExecutorImpl.processNotifies(QueryExecutorImpl. >java:547) - waiting to lock <0xb1a033e0> (a > org.postgresql.core.v3.QueryExecutorImpl) at > org.postgresql.core.v3.ProtocolConnectionImpl.getNotifications(ProtocolConn >ectionImpl.java:68) - locked <0xb1a00f88> (a > org.postgresql.core.v3.ProtocolConnectionImpl) at > org.postgresql.jdbc2.AbstractJdbc2Connection.getNotifications(AbstractJdbc2 >Connection.java:1013) at > database.DbListenerHandler$DbListener.run(DbListenerHandler.java:134) at > java.lang.Thread.run(Thread.java:619) > > Found 1 deadlock. > > I've marked the places where the deadlock occurs with "\\<<<<<<<<<<<". > > In the "DB connection select 1" thread I have: > ==================================== > success = false; > Statement stmt = null; > try { > int i = 0; > stmt = getCon().createStatement(); > ResultSet rs = stmt.executeQuery("SELECT 1");\\<<<<<<<<<<< > > if (rs.next()) { > i = rs.getInt(1); > } > if (i == 1) { > success = true; > } > } catch (SQLException ex) { > } finally { > if (stmt != null) { > try { > stmt.close(); > } catch (SQLException ex) { > } > } > } > > ==================================== > > And in the "Database tables listener" thread I have: > ==================================== > while (...) { > try { > PGNotification notifications[] > =pgconn.getNotifications();\\<<<<<<<<<<< if (notifications != null) { > .... > } > Thread.sleep(300); > } catch (SQLException sqle) { > .... > } catch (InterruptedException ie) { > .... > } > } > ==================================== > > Is this a bug in jdbc-postgres? > > Joao Leal > > - > Sent via pgsql-jdbc mailing list (pgsql-jdbc@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-jdbc -- Engº de Processo // Ciengis - Sistemas de Controlo Avançado, Lda IPN, Rua Pedro Nunes 3030-199 Coimbra PORTUGAL Tel/Fax: +351 239 700 353 / 301 Telm.: +351 934 699 919 www.ciengis.com
On Tue, 25 Mar 2008, Joao Rui Leal wrote: > I did a workaround/hack to fix the problem, but there should be some better > way to fix this. I had to make sure that the locking order in > getNotifications() is same as in executeQuery(), but that meant exposing the > QueryExecutor in the ProtocolConnection (the QueryExecutorImpl is saved as > private variable inside ProtocolConnectionImpl). So I had to make the > following changes (it's not pretty, I know...): I don't see why you need access to the Impl version. Isn't "synchronized (getQueryExecutor())" around AbstractJdbc2Connection's protoConnection.getNotifications() sufficient? Still that's not a real clean/understandable design. Perhaps instead processNotifies() should be added to the public QueryExecutor interface and then AbstractJdbc2Connection can call processNotifies itself so that fetching notifications from protoConnection doesn't require any interaction with the QueryExecutor. Kris Jurka
On Tuesday 25 March 2008, Kris Jurka wrote: > On Tue, 25 Mar 2008, Joao Rui Leal wrote: > > I did a workaround/hack to fix the problem, but there should be some > > better way to fix this. I had to make sure that the locking order in > > getNotifications() is same as in executeQuery(), but that meant exposing > > the QueryExecutor in the ProtocolConnection (the QueryExecutorImpl is > > saved as private variable inside ProtocolConnectionImpl). So I had to > > make the following changes (it's not pretty, I know...): > > I don't see why you need access to the Impl version. Isn't "synchronized > (getQueryExecutor())" around AbstractJdbc2Connection's > protoConnection.getNotifications() sufficient? I haven't tested it yet but you seem to be right. I didn't know there was such a method (1st time going through the API). I guess if it returns the org.postgresql.core.v3.QueryExecutorImpl object then the locking will be in the same order as in executeQuery(). > > Still that's not a real clean/understandable design. Perhaps instead > processNotifies() should be added to the public QueryExecutor interface > and then AbstractJdbc2Connection can call processNotifies itself so that > fetching notifications from protoConnection doesn't require any > interaction with the QueryExecutor. I agree. Joao Leal > > Kris Jurka
Joao Rui Leal wrote: > On Tuesday 25 March 2008, Kris Jurka wrote: >> Still that's not a real clean/understandable design. Perhaps instead >> processNotifies() should be added to the public QueryExecutor interface >> and then AbstractJdbc2Connection can call processNotifies itself so that >> fetching notifications from protoConnection doesn't require any >> interaction with the QueryExecutor. > > I agree. > I've applied the attached patch to CVS back to the 8.1 driver. The 8.0 driver does not have this problem because it doesn't allow notification retrieval without executing a query. Thanks for the report and diagnosis. Kris Jurka Index: org/postgresql/core/QueryExecutor.java =================================================================== RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/core/QueryExecutor.java,v retrieving revision 1.41 diff -c -r1.41 QueryExecutor.java *** org/postgresql/core/QueryExecutor.java 8 Jan 2008 06:56:27 -0000 1.41 --- org/postgresql/core/QueryExecutor.java 1 Apr 2008 07:09:38 -0000 *************** *** 170,175 **** --- 170,184 ---- */ Query createParameterizedQuery(String sql); // Parsed for parameter placeholders ('?') + /** + * Prior to attempting to retrieve notifications, we need to pull + * any recently received notifications off of the network buffers. + * The notification retrieval in ProtocolConnection cannot do this + * as it is prone to deadlock, so the higher level caller must be + * responsible which requires exposing this method. + */ + void processNotifies() throws SQLException; + // // Fastpath interface. // Index: org/postgresql/core/v2/ProtocolConnectionImpl.java =================================================================== RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/core/v2/ProtocolConnectionImpl.java,v retrieving revision 1.11 diff -c -r1.11 ProtocolConnectionImpl.java *** org/postgresql/core/v2/ProtocolConnectionImpl.java 8 Jan 2008 06:56:27 -0000 1.11 --- org/postgresql/core/v2/ProtocolConnectionImpl.java 1 Apr 2008 07:09:39 -0000 *************** *** 62,68 **** } public synchronized PGNotification[] getNotifications() throws SQLException { - executor.processNotifies(); PGNotification[] array = (PGNotification[])notifications.toArray(new PGNotification[notifications.size()]); notifications.clear(); return array; --- 62,67 ---- Index: org/postgresql/core/v3/ProtocolConnectionImpl.java =================================================================== RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/core/v3/ProtocolConnectionImpl.java,v retrieving revision 1.12 diff -c -r1.12 ProtocolConnectionImpl.java *** org/postgresql/core/v3/ProtocolConnectionImpl.java 8 Jan 2008 06:56:27 -0000 1.12 --- org/postgresql/core/v3/ProtocolConnectionImpl.java 1 Apr 2008 07:09:39 -0000 *************** *** 65,71 **** } public synchronized PGNotification[] getNotifications() throws SQLException { - executor.processNotifies(); PGNotification[] array = (PGNotification[])notifications.toArray(new PGNotification[notifications.size()]); notifications.clear(); return array; --- 65,70 ---- Index: org/postgresql/jdbc2/AbstractJdbc2Connection.java =================================================================== RCS file: /cvsroot/jdbc/pgjdbc/org/postgresql/jdbc2/AbstractJdbc2Connection.java,v retrieving revision 1.48 diff -c -r1.48 AbstractJdbc2Connection.java *** org/postgresql/jdbc2/AbstractJdbc2Connection.java 19 Feb 2008 06:12:24 -0000 1.48 --- org/postgresql/jdbc2/AbstractJdbc2Connection.java 1 Apr 2008 07:09:39 -0000 *************** *** 1022,1027 **** --- 1022,1028 ---- public PGNotification[] getNotifications() throws SQLException { + getQueryExecutor().processNotifies(); // Backwards-compatibility hand-holding. PGNotification[] notifications = protoConnection.getNotifications(); return (notifications.length == 0 ? null : notifications);