[jboss-jira] [JBoss JIRA] (JBJCA-1338) CheckValidConnectionSQL can open a transaction, preventing application from changing transaction isolation level (PostgreSQL)

[Tomas Hofman (JIRA)]

    [ https://issues.jboss.org/browse/JBJCA-1338?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13387331#comment-13387331 ] 

Tomas Hofman commented on JBJCA-1338:
-------------------------------------

Commit was rolled back due to this regression: https://issues.jboss.org/browse/JBEAP-9730

Some analysis:

# The change I'd made set {{autocommit = true}} on the jdbc connection and reset the {{underlyingAutoCommit}} var in the {{BaseWrapperManagedConnection#cleanup()}} method:
  {code}
BaseWrapperManagedConnection#cleanup():

          jdbcAutoCommit = true;
+         if (jdbcAutoCommit != underlyingAutoCommit)
+         {
+            try {
+               con.setAutoCommit(jdbcAutoCommit);
+               underlyingAutoCommit = jdbcAutoCommit;
+            } catch (SQLException e) {
+               mcf.log.errorResettingAutoCommit(mcf.getJndiName(), e);
+            }
+         }
  {code}
# That broke following fix in the {{destroy()}} method, which depended on {{underlyingAutoCommit}} var:
  {code}
BaseWrapperManagedConnection#destroy():

         // See JBAS-5678
         if (!underlyingAutoCommit)
            con.rollback();
  {code}
  so the connection was not rolled back on destroy caused by a connection error and the transaction somehow got committed.
  *But* that actually wasn't the reason why the test failed, as this fix is probably only relevant to Oracle db, which apparently commits a transaction when closing the connection (JBAS-5678).
# The reason of the test failure is that postgresql driver commits a running transaction when autocommit is set to true:
  {code}
PgConnection:

  public void setAutoCommit(boolean autoCommit) throws SQLException {
    checkClosed();

    if (this.autoCommit == autoCommit) {
      return;
    }

    if (!this.autoCommit) {
      commit();
    }

    this.autoCommit = autoCommit;
  }
  {code}
# Also related - normally if a transaction is left open and a managed connection is closed, the transaction is closed in {{TxConnectionListener.tidyup()}}. That doesn't happen if the connection is closed due to connection error:
  {code}
AbstractConnectionManager#returnManagedConnection():

         if (!kill && cl.getState().equals(ConnectionState.NORMAL))
         {
            cl.tidyup();
         }
  {code}

Possible solutions:

# Continue with the old fix, just add {{con.rollback()}} before {{con.setAutoCommit(true)}} in the {{cleanup()}} method.
# Rather then in the cleanup method, we could reset autocommit state in {{BaseWrapperManagedConnectionFactory#getInvalidConnections()}} just before validating the connection, where there should be no running transaction present...

> CheckValidConnectionSQL can open a transaction, preventing application from changing transaction isolation level (PostgreSQL)
> -----------------------------------------------------------------------------------------------------------------------------
>
>                 Key: JBJCA-1338
>                 URL: https://issues.jboss.org/browse/JBJCA-1338
>             Project: IronJacamar
>          Issue Type: Bug
>          Components: JDBC
>    Affects Versions: 1.2.7.Final
>            Reporter: Tomas Hofman
>            Assignee: Tomas Hofman
>
> PostgreSQL driver only allows changing the transaction isolation level when transaction is not opened. Under certain circumstances, an application can receive a connection with already opened transaction and an attempt to change transaction isolation level will lead to exception.
> This happens with the PostgreSQL driver and with CheckValidConnectionSQL checker configured to run a select statement to verify connections retrieved from the pool.
> The scenario is as follows:
> # A connection is retrieved from the pool for the 1st app and CheckValidConnectionSQL verifies it by running a select statement (autocommit is set to true by default). This statement is run directly via the jdbc connection, not the wrapper.
> # 1st app receives the connection, sets autocommit=false, perform some work and commits a transaction.
> # The connection is returned to the pool, {{cleanup()}} method is called on LocalManagedConnection wrapper, which sets autocommit=true. This however doesn't reset autocommit on the wrapped jdbc connection yet, which would only happen just before executing another SQL statement f.i.
> # The same connection is retrieved from the pool for the 2nd app and CheckValidConnectionSQL runs the query. Because the jdbc connection has still autocommit=false, new transaction is opened.
> # 2nd app receives the connection and calls {{setTransactionIsolation()}}, which throws an exception because the transaction is open.
> Possible solution could be that the {{cleanup()}} method propagates the autocommit=true to the wrapped jdbc connection immediately.



--
This message was sent by Atlassian JIRA
(v7.2.3#72005)