JDBC-386 Changes to transaction configuration of a connection should …

…not be propagated to all connections with same properties Backported from Jaybird 5
FirebirdSQL · Mar 29, 2020 · 70e970f · 70e970f
1 parent dd6e601
commit 70e970f
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 29 deletions.
diff --git a/src/documentation/release_notes.md b/src/documentation/release_notes.md
@@ -246,18 +246,26 @@ Jaybird 3.0.x changelog
 Changes in Jaybird 3.0.9
 ------------------------
 
-The following has been changed or fixed since Jaybird 3.0.9:
-
--   New feature: Jaybird now supports UTF-8 URL encoding for connection
-    properties in the JDBC url. ([JDBC-604](http://tracker.firebirdsql.org/browse/JDBC-604)) \
-    This introduce a minor incompatibility, see also 
-    [URL encoding in query part of JDBC URL]. \
+The following has been changed or fixed since Jaybird 3.0.8:
+
+-   Fixed: changes to the transaction configuration (transaction parameter
+    buffer configuration) of one connection are no longer propagated to other
+    connections with the same connection properties ([JDBC-386](http://tracker.firebirdsql.org/browse/JDBC-386)) \
+    This change introduce a binary incompatibility as method 
+    `setTransactionParameters(int, TransactionParameterBuffer)` in
+    `FBManagedConnection` can now throw `ResourceException` where previously it
+    did not. Under the assumption that most users of Jaybird are not directly
+    using this class, the change should not break anything.
 -   New feature: Firebird 4 data type bind configuration support ([JDBC-603](http://tracker.firebirdsql.org/browse/JDBC-603)) \
     This change also removes the `timeZoneBind` and `decfloatBind` connection
     properties introduced in Jaybird 3.0.6 as the corresponding DPB items were
     removed from Firebird 4. This feature requires
     Firebird 4 beta 2 or snapshot Firebird 4.0.0.1683 or higher. \
-    See also [Limited support for new Firebird 4 data types]. \
+    See also [Limited support for new Firebird 4 data types].
+-   New feature: Jaybird now supports UTF-8 URL encoding for connection
+    properties in the JDBC url. ([JDBC-604](http://tracker.firebirdsql.org/browse/JDBC-604)) \
+    This introduce a minor incompatibility, see also 
+    [URL encoding in query part of JDBC URL].
 
 ### Known issues in Jaybird 3.0.9
 

diff --git a/src/main/org/firebirdsql/jca/FBManagedConnection.java b/src/main/org/firebirdsql/jca/FBManagedConnection.java
@@ -86,6 +86,7 @@ public class FBManagedConnection implements ManagedConnection, XAResource, Excep
     private final Object syncObject;
 
     private final FBConnectionRequestInfo cri;
+    private FBTpbMapper transactionMapping;
     private FBTpb tpb;
     private int transactionIsolation;
 
@@ -444,16 +445,19 @@ public void associateConnection(Object connection) throws ResourceException {
      */
     public void cleanup() throws ResourceException {
         disassociateConnections();
+        synchronized (syncObject) {
+            try {
+                getGDSHelper().setCurrentTransaction(null);
+            } catch (SQLException e) {
+                throw new FBResourceException(e);
+            }
 
-        try {
-            getGDSHelper().setCurrentTransaction(null);
-        } catch (SQLException e) {
-            throw new FBResourceException(e);
+            // reset the transaction mapping to use the default of the MCF
+            transactionMapping = null;
+            // reset the TPB from the previous transaction.
+            this.tpb = mcf.getDefaultTpb();
+            this.transactionIsolation = mcf.getDefaultTransactionIsolation();
         }
-
-        // reset the TPB from the previous transaction.
-        this.tpb = mcf.getDefaultTpb();
-        this.transactionIsolation = mcf.getDefaultTransactionIsolation();
     }
 
     /**
@@ -1238,19 +1242,40 @@ public FBConnectionRequestInfo getConnectionRequestInfo() {
     }
 
     public TransactionParameterBuffer getTransactionParameters() {
-        return tpb.getTransactionParameterBuffer();
+        synchronized (syncObject) {
+            return tpb.getTransactionParameterBuffer();
+        }
     }
 
     public void setTransactionParameters(TransactionParameterBuffer transactionParameters) {
-        tpb.setTransactionParameterBuffer(transactionParameters);
+        synchronized (syncObject) {
+            tpb.setTransactionParameterBuffer(transactionParameters);
+        }
     }
 
     public TransactionParameterBuffer getTransactionParameters(int isolation) {
-        return mcf.getTransactionParameters(isolation);
+        synchronized (syncObject) {
+            final FBTpbMapper mapping = transactionMapping;
+            if (mapping == null) {
+                return mcf.getTransactionParameters(isolation);
+            }
+            return mapping.getMapping(isolation);
+        }
     }
 
-    public void setTransactionParameters(int isolation, TransactionParameterBuffer transactionParams) {
-        mcf.setTransactionParameters(isolation, transactionParams);
+    public void setTransactionParameters(int isolation, TransactionParameterBuffer transactionParams)
+            throws ResourceException {
+        synchronized (syncObject) {
+            FBTpbMapper mapping = transactionMapping;
+            if (mapping == null) {
+                mapping = transactionMapping = mcf.getTransactionMappingCopy();
+            }
+            mapping.setMapping(isolation, transactionParams);
+            if (getTransactionIsolation() == isolation) {
+                // Make sure next transaction uses the new config
+                setTransactionIsolation(isolation);
+            }
+        }
     }
 
     // --------------------------------------------------------------------
@@ -1370,7 +1395,9 @@ boolean matches(Subject subj, ConnectionRequestInfo cri) {
      *             If the transaction level cannot be retrieved
      */
     public int getTransactionIsolation() throws ResourceException {
-        return transactionIsolation;
+        synchronized (syncObject) {
+            return transactionIsolation;
+        }
     }
 
     /**
@@ -1390,9 +1417,13 @@ public int getTransactionIsolation() throws ResourceException {
      *             If the transaction level cannot be retrieved
      */
     public void setTransactionIsolation(int isolation) throws ResourceException {
-        transactionIsolation = isolation;
-
-        tpb = mcf.getTpb(isolation);
+        synchronized (syncObject) {
+            transactionIsolation = isolation;
+            final FBTpbMapper mapping = transactionMapping;
+            tpb = mapping == null
+                    ? mcf.getTpb(isolation)
+                    : new FBTpb(mapping.getMapping(isolation));
+        }
     }
 
     /**

diff --git a/src/main/org/firebirdsql/jca/FBManagedConnectionFactory.java b/src/main/org/firebirdsql/jca/FBManagedConnectionFactory.java
@@ -30,10 +30,7 @@
 import org.firebirdsql.gds.ng.FbStatement;
 import org.firebirdsql.gds.ng.FbTransaction;
 import org.firebirdsql.gds.ng.fields.RowValue;
-import org.firebirdsql.jdbc.FBConnection;
-import org.firebirdsql.jdbc.FBConnectionProperties;
-import org.firebirdsql.jdbc.FBDataSource;
-import org.firebirdsql.jdbc.FirebirdConnectionProperties;
+import org.firebirdsql.jdbc.*;
 
 import javax.resource.NotSupportedException;
 import javax.resource.ResourceException;
@@ -490,6 +487,17 @@ public FBTpb getTpb(int defaultTransactionIsolation) throws FBResourceException
                 defaultTransactionIsolation));
     }
 
+    /**
+     * Get a copy of the current transaction mapping.
+     *
+     * @return Copy of the transaction mapping
+     * @throws ResourceException
+     *         For errors on obtaining or creating the transaction mapping
+     */
+    FBTpbMapper getTransactionMappingCopy() throws ResourceException {
+        return (FBTpbMapper) connectionProperties.getMapper().clone();
+    }
+
     /**
      * The <code>createConnectionFactory</code> method creates a DataSource
      * using the supplied ConnectionManager.

diff --git a/src/main/org/firebirdsql/jdbc/FBConnection.java b/src/main/org/firebirdsql/jdbc/FBConnection.java
@@ -257,7 +257,11 @@ public void setTransactionParameters(int isolationLevel, TransactionParameterBuf
                 throw new FBSQLException("Cannot set transaction parameters in managed environment.");
             }
 
-            mc.setTransactionParameters(isolationLevel, tpb);
+            try {
+                mc.setTransactionParameters(isolationLevel, tpb);
+            } catch (ResourceException ex) {
+                throw new FBSQLException(ex);
+            }
         }
     }
 

diff --git a/src/test/org/firebirdsql/jdbc/TestFBConnection.java b/src/test/org/firebirdsql/jdbc/TestFBConnection.java
@@ -25,6 +25,7 @@
 import org.firebirdsql.gds.JaybirdErrorCodes;
 import org.firebirdsql.gds.TransactionParameterBuffer;
 import org.firebirdsql.gds.impl.GDSServerVersion;
+import org.firebirdsql.gds.impl.TransactionParameterBufferImpl;
 import org.firebirdsql.gds.impl.jni.NativeGDSFactoryPlugin;
 import org.firebirdsql.gds.impl.oo.OOGDSFactoryPlugin;
 import org.firebirdsql.gds.impl.wire.WireGDSFactoryPlugin;
@@ -710,4 +711,30 @@ public void connectingWithUnknownJavaCharacterSetName() throws Exception {
         }
     }
 
+    /**
+     * Rationale: see <a href="http://tracker.firebirdsql.org/browse/JDBC-386">JDBC-386</a>
+     */
+    @Test
+    public void transactionSettingsNotShared() throws Exception {
+        try (FBConnection con1 = getConnectionViaDriverManager().unwrap(FBConnection.class);
+             FBConnection con2 = getConnectionViaDriverManager().unwrap(FBConnection.class)) {
+            TransactionParameterBuffer con2Original =
+                    con2.getTransactionParameters(Connection.TRANSACTION_REPEATABLE_READ);
+
+            TransactionParameterBufferImpl newParameters = new TransactionParameterBufferImpl();
+            newParameters.addArgument(TransactionParameterBuffer.CONSISTENCY);
+            newParameters.addArgument(TransactionParameterBuffer.READ);
+            newParameters.addArgument(TransactionParameterBuffer.NOWAIT);
+
+            con1.setTransactionParameters(Connection.TRANSACTION_REPEATABLE_READ, newParameters);
+
+            assertEquals("Setting of con1 update",
+                    newParameters, con1.getTransactionParameters(Connection.TRANSACTION_REPEATABLE_READ));
+            assertEquals("Setting of con2 unchanged",
+                    con2Original, con2.getTransactionParameters(Connection.TRANSACTION_REPEATABLE_READ));
+            assertNotEquals("Setting of con2 not equal to new config of con1",
+                    newParameters, con2.getTransactionParameters(Connection.TRANSACTION_REPEATABLE_READ));
+        }
+    }
+
 }