From dae2b07610205264fb85c0021684b1f9d563773c Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Thu, 26 Feb 2015 21:27:39 +0000 Subject: [PATCH] WFLY-3619 Added a TransactionSynchronizationRegistry that can order the JCA Synchronizations relative to other interposed Synchronizations --- .../subsystems/jca/JcaExtension.java | 3 - .../as/txn/logging/TransactionLogger.java | 15 ++ .../ArjunaTransactionManagerService.java | 5 +- .../JCAOrderedLastSynchronizationList.java | 168 ++++++++++++++++++ ...sactionSynchronizationRegistryWrapper.java | 115 ++++++++++++ 5 files changed, 301 insertions(+), 5 deletions(-) create mode 100644 transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/JCAOrderedLastSynchronizationList.java create mode 100644 transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/TransactionSynchronizationRegistryWrapper.java diff --git a/connector/src/main/java/org/jboss/as/connector/subsystems/jca/JcaExtension.java b/connector/src/main/java/org/jboss/as/connector/subsystems/jca/JcaExtension.java index a959e1e0aca4..57456a6b0fa3 100644 --- a/connector/src/main/java/org/jboss/as/connector/subsystems/jca/JcaExtension.java +++ b/connector/src/main/java/org/jboss/as/connector/subsystems/jca/JcaExtension.java @@ -106,9 +106,6 @@ public void initialize(final ExtensionContext context) { if (context.isRegisterTransformers()) { JcaSubsystemRootDefinition.registerTransformers(subsystem); } - - // Workaround for WFLY-3619, until we have correct explicit ordering - System.setProperty("ironjacamar.no_delist_resource_all", "true"); } @Override diff --git a/transactions/src/main/java/org/jboss/as/txn/logging/TransactionLogger.java b/transactions/src/main/java/org/jboss/as/txn/logging/TransactionLogger.java index 7b856ec050da..80051a299577 100644 --- a/transactions/src/main/java/org/jboss/as/txn/logging/TransactionLogger.java +++ b/transactions/src/main/java/org/jboss/as/txn/logging/TransactionLogger.java @@ -27,6 +27,9 @@ import java.io.IOException; +import javax.transaction.Synchronization; +import javax.transaction.Transaction; + import org.jboss.as.controller.OperationFailedException; import org.jboss.logging.BasicLogger; import org.jboss.logging.Logger; @@ -196,4 +199,16 @@ public interface TransactionLogger extends BasicLogger { @Message(id = 25, value = "Either %s must be 'true' or %s must be defined.") OperationFailedException eitherTrueOrDefined(String attrOne, String attrTwo); + + @LogMessage(level = WARN) + @Message(id = 26, value = "The transaction %s could not be removed from the cache during cleanup.") + void transactionNotFound(Transaction tx); + + @LogMessage(level = WARN) + @Message(id = 27, value = "The pre-jca synchronization %s associated with tx %s failed during after completion") + void preJcaSyncAfterCompletionFailed(Synchronization preJcaSync, Transaction tx, @Cause Exception e); + + @LogMessage(level = WARN) + @Message(id = 28, value = "The jca synchronization %s associated with tx %s failed during after completion") + void jcaSyncAfterCompletionFailed(Synchronization jcaSync, Transaction tx, @Cause Exception e); } diff --git a/transactions/src/main/java/org/jboss/as/txn/service/ArjunaTransactionManagerService.java b/transactions/src/main/java/org/jboss/as/txn/service/ArjunaTransactionManagerService.java index d82f3790fa0d..44585a9ee7b3 100644 --- a/transactions/src/main/java/org/jboss/as/txn/service/ArjunaTransactionManagerService.java +++ b/transactions/src/main/java/org/jboss/as/txn/service/ArjunaTransactionManagerService.java @@ -28,6 +28,7 @@ import com.arjuna.ats.jta.common.JTAEnvironmentBean; import com.arjuna.orbportability.internal.utils.PostInitLoader; import org.jboss.as.txn.logging.TransactionLogger; +import org.jboss.as.txn.service.internal.tsr.TransactionSynchronizationRegistryWrapper; import org.jboss.msc.inject.Injector; import org.jboss.msc.service.Service; import org.jboss.msc.service.ServiceName; @@ -105,7 +106,7 @@ public synchronized void start(final StartContext context) throws StartException userTransactionRegistry.getValue().addProvider(userTransaction); jtaEnvironmentBean.getValue().setUserTransaction(userTransaction); service.setJbossXATerminator(xaTerminatorInjector.getValue()); - service.setTransactionSynchronizationRegistry(new com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionSynchronizationRegistryImple()); + service.setTransactionSynchronizationRegistry(new TransactionSynchronizationRegistryWrapper( new com.arjuna.ats.internal.jta.transaction.arjunacore.TransactionSynchronizationRegistryImple())); try { service.create(); @@ -127,7 +128,7 @@ public synchronized void start(final StartContext context) throws StartException jtaEnvironmentBean.getValue().setUserTransaction(userTransaction); userTransactionRegistry.getValue().addProvider(userTransaction); service.setJbossXATerminator(xaTerminatorInjector.getValue()); - service.setTransactionSynchronizationRegistry(new com.arjuna.ats.internal.jta.transaction.jts.TransactionSynchronizationRegistryImple()); + service.setTransactionSynchronizationRegistry(new TransactionSynchronizationRegistryWrapper( new com.arjuna.ats.internal.jta.transaction.jts.TransactionSynchronizationRegistryImple())); objStoreBrowserTypes.put("StateManager/BasicAction/TwoPhaseCoordinator/ArjunaTransactionImple", "com.arjuna.ats.arjuna.tools.osb.mbean.ActionBean"); diff --git a/transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/JCAOrderedLastSynchronizationList.java b/transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/JCAOrderedLastSynchronizationList.java new file mode 100644 index 000000000000..5c65fb16e45b --- /dev/null +++ b/transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/JCAOrderedLastSynchronizationList.java @@ -0,0 +1,168 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2015, Red Hat, Inc., and individual contributors + * as indicated by the @author tags. See the copyright.txt file in the + * distribution for a full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.jboss.as.txn.service.internal.tsr; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; + +import javax.transaction.Synchronization; +import javax.transaction.Transaction; + +import org.jboss.as.txn.logging.TransactionLogger; + +/** + * This class was added to: + * + * 1. workaround an issue discussed in https://java.net/jira/browse/JTA_SPEC-4 whereby the JCA Synchronization(s) need to be + * called after the JPA Synchronization(s). Currently the implementation orders JCA relative to all interposed Synchronizations, + * if this is not desirable it would be possible to modify this class to store just the JPA and JCA syncs and the other syncs + * can simply be passed to a delegate (would need the reference to this in the constructor). + * + * 2. During afterCompletion the JCA synchronizations should be called last as that allows JCA to detect connection leaks from + * frameworks that have not closed the JCA managed resources. This is described in (for example) + * http://docs.oracle.com/javaee/5/api/javax/transaction/TransactionSynchronizationRegistry + * .html#registerInterposedSynchronization(javax.transaction.Synchronization) where it says that during afterCompletion + * "Resources can be closed but no transactional work can be performed with them" + */ +public class JCAOrderedLastSynchronizationList implements Synchronization { + private Transaction tx; + private Map jcaOrderedLastSynchronizations; + private List preJcaSyncs = new ArrayList(); + private List jcaSyncs = new ArrayList(); + + public JCAOrderedLastSynchronizationList(Transaction tx, + Map jcaOrderedLastSynchronizations) { + this.tx = tx; + this.jcaOrderedLastSynchronizations = jcaOrderedLastSynchronizations; + } + + public void addInterposedSynchronization(Synchronization synchronization) { + if (synchronization.getClass().getName().startsWith("org.jboss.jca")) { + if (TransactionLogger.ROOT_LOGGER.isTraceEnabled()) { + TransactionLogger.ROOT_LOGGER.trace("JCAOrderedLastSynchronizationList.jcaSyncs.add - Class: " + synchronization.getClass() + " HashCode: " + + synchronization.hashCode() + " toString: " + synchronization); + } + jcaSyncs.add(synchronization); + } else { + if (TransactionLogger.ROOT_LOGGER.isTraceEnabled()) { + TransactionLogger.ROOT_LOGGER.trace("JCAOrderedLastSynchronizationList.preJcaSyncs.add - Class: " + synchronization.getClass() + " HashCode: " + + synchronization.hashCode() + " toString: " + synchronization); + } + preJcaSyncs.add(synchronization); + } + } + + /** + * Exceptions from Synchronizations that are registered with this TSR are not trapped for before completion. This is because + * an error in a Sync here should result in the transaction rolling back. + * + * You can see that in effect in these classes: + * https://github.com/jbosstm/narayana/blob/5.0.4.Final/ArjunaCore/arjuna/classes + * /com/arjuna/ats/arjuna/coordinator/TwoPhaseCoordinator.java#L91 + * https://github.com/jbosstm/narayana/blob/5.0.4.Final/ArjunaJTA + * /jta/classes/com/arjuna/ats/internal/jta/resources/arjunacore/SynchronizationImple.java#L76 + */ + @Override + public void beforeCompletion() { + for (Synchronization preJcaSync : preJcaSyncs) { + if (TransactionLogger.ROOT_LOGGER.isTraceEnabled()) { + TransactionLogger.ROOT_LOGGER.trace("JCAOrderedLastSynchronizationList.preJcaSyncs.before_completion - Class: " + preJcaSync.getClass() + " HashCode: " + + preJcaSync.hashCode() + + " toString: " + + preJcaSync); + } + preJcaSync.beforeCompletion(); + } + for (Synchronization jcaSync : jcaSyncs) { + if (TransactionLogger.ROOT_LOGGER.isTraceEnabled()) { + TransactionLogger.ROOT_LOGGER.trace("JCAOrderedLastSynchronizationList.jcaSyncs.before_completion - Class: " + jcaSync.getClass() + " HashCode: " + + jcaSync.hashCode() + + " toString: " + + jcaSync); + } + jcaSync.beforeCompletion(); + } + } + + @Override + public void afterCompletion(int status) { + // The list should be iterated in reverse order - has issues with EJB3 if not + // https://github.com/jbosstm/narayana/blob/master/ArjunaCore/arjuna/classes/com/arjuna/ats/arjuna/coordinator/TwoPhaseCoordinator.java#L509 + Collections.reverse(preJcaSyncs); + for (Synchronization preJcaSync : preJcaSyncs) { + if (TransactionLogger.ROOT_LOGGER.isTraceEnabled()) { + TransactionLogger.ROOT_LOGGER.trace("JCAOrderedLastSynchronizationList.preJcaSyncs.afterCompletion - Class: " + preJcaSync.getClass() + " HashCode: " + + preJcaSync.hashCode() + + " toString: " + preJcaSync); + } + try { + preJcaSync.afterCompletion(status); + } catch (Exception e) { + // Trap these exceptions so the rest of the synchronizations get the chance to complete + // https://github.com/jbosstm/narayana/blob/5.0.4.Final/ArjunaJTA/jta/classes/com/arjuna/ats/internal/jta/resources/arjunacore/SynchronizationImple.java#L102 + TransactionLogger.ROOT_LOGGER.preJcaSyncAfterCompletionFailed(preJcaSync, tx, e); + } + } + Collections.reverse(jcaSyncs); + for (Synchronization jcaSync : jcaSyncs) { + if (TransactionLogger.ROOT_LOGGER.isTraceEnabled()) { + TransactionLogger.ROOT_LOGGER.trace("JCAOrderedLastSynchronizationList.jcaSyncs.afterCompletion - Class: " + jcaSync.getClass() + " HashCode: " + + jcaSync.hashCode() + + " toString: " + + jcaSync); + } + try { + jcaSync.afterCompletion(status); + } catch (Exception e) { + // Trap these exceptions so the rest of the synchronizations get the chance to complete + // https://github.com/jbosstm/narayana/blob/5.0.4.Final/ArjunaJTA/jta/classes/com/arjuna/ats/internal/jta/resources/arjunacore/SynchronizationImple.java#L102 + TransactionLogger.ROOT_LOGGER.jcaSyncAfterCompletionFailed(jcaSync, tx, e); + } + } + + if (jcaOrderedLastSynchronizations.remove(tx) == null) { + // The identifier wasn't stable - scan for it - this can happen in JTS propagation when the UID needs retrieving + // from the parent and the parent has been deactivated + Transaction altKey = null; + Iterator> iterator = jcaOrderedLastSynchronizations.entrySet().iterator(); + while (altKey == null && iterator.hasNext()) { + Map.Entry next = iterator.next(); + if (next.getValue().equals(this)) { + altKey = next.getKey(); + iterator.remove(); + if (TransactionLogger.ROOT_LOGGER.isTraceEnabled()) { + TransactionLogger.ROOT_LOGGER.tracef("Removed: %s [%s]", System.identityHashCode(tx), tx.toString()); + } + break; + } + } + + if (altKey == null) { + TransactionLogger.ROOT_LOGGER.transactionNotFound(tx); + } + } + } +} diff --git a/transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/TransactionSynchronizationRegistryWrapper.java b/transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/TransactionSynchronizationRegistryWrapper.java new file mode 100644 index 000000000000..65b7326afa15 --- /dev/null +++ b/transactions/src/main/java/org/jboss/as/txn/service/internal/tsr/TransactionSynchronizationRegistryWrapper.java @@ -0,0 +1,115 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2015, Red Hat, Inc., and individual contributors + * as indicated by the @author tags. See the copyright.txt file in the + * distribution for a full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.jboss.as.txn.service.internal.tsr; + +import java.util.concurrent.ConcurrentHashMap; + +import javax.transaction.Synchronization; +import javax.transaction.SystemException; +import javax.transaction.Transaction; +import javax.transaction.TransactionManager; +import javax.transaction.TransactionSynchronizationRegistry; + +/** + * Most of this implementation delegates down to the underlying transactions implementation to provide the services of the + * TransactionSynchronizationRegistry. The one area it modifies is the registration of the interposed Synchronizations. The + * reason this implementation needs to differ is because the JCA Synchronization and JPA Synchronizations are both specified as + * Interposed however there are defined ordering requirements between them both. + * + * The current implementation orders JCA relative to all other Synchronizations. For beforeCompletion, it would be possible to + * restrict this to the one case where JCA is ordered before JPA, however it is possible that other interposed Synchronizations + * would require the services of JCA and as such if the JCA is allowed to execute delistResource during beforeCompletion as + * mandated in JCA spec the behaviour of those subsequent interactions would be broken. For afterCompletion the JCA + * synchronizations are called last as that allows JCA to detect connection leaks from frameworks that have not closed the JCA + * managed resources. This is described in (for example) + * http://docs.oracle.com/javaee/5/api/javax/transaction/TransactionSynchronizationRegistry + * .html#registerInterposedSynchronization(javax.transaction.Synchronization) where it says that during afterCompletion + * "Resources can be closed but no transactional work can be performed with them". + * + * One implication of this approach is that if the underlying transactions implementation has special handling for various types + * of Synchronization that can also implement other interfaces (i.e. if interposedSync instanceof OtherInterface) these + * behaviours cannot take effect as the underlying implementation will never directly see the actual Synchronizations. + */ +public class TransactionSynchronizationRegistryWrapper implements TransactionSynchronizationRegistry { + + private TransactionSynchronizationRegistry delegate; + private TransactionManager transactionManager; + private ConcurrentHashMap interposedSyncs = new ConcurrentHashMap(); + + public TransactionSynchronizationRegistryWrapper(TransactionSynchronizationRegistry delegate) { + this.delegate = delegate; + transactionManager = com.arjuna.ats.jta.TransactionManager + .transactionManager(); + } + + @Override + public void registerInterposedSynchronization(Synchronization sync) + throws IllegalStateException { + try { + Transaction tx = transactionManager.getTransaction(); + JCAOrderedLastSynchronizationList jcaOrderedLastSynchronization = interposedSyncs.get(tx); + if (jcaOrderedLastSynchronization == null) { + JCAOrderedLastSynchronizationList toPut = new JCAOrderedLastSynchronizationList(tx, interposedSyncs); + jcaOrderedLastSynchronization = interposedSyncs.putIfAbsent(tx, toPut); + if (jcaOrderedLastSynchronization == null) { + jcaOrderedLastSynchronization = toPut; + delegate.registerInterposedSynchronization(jcaOrderedLastSynchronization); + } + } + jcaOrderedLastSynchronization.addInterposedSynchronization(sync); + } catch (SystemException e) { + throw new IllegalStateException(e); + } + } + + @Override + public Object getTransactionKey() { + return delegate.getTransactionKey(); + } + + @Override + public int getTransactionStatus() { + return delegate.getTransactionStatus(); + } + + @Override + public boolean getRollbackOnly() throws IllegalStateException { + return delegate.getRollbackOnly(); + } + + @Override + public void setRollbackOnly() throws IllegalStateException { + delegate.setRollbackOnly(); + } + + @Override + public Object getResource(Object key) throws IllegalStateException { + return delegate.getResource(key); + } + + @Override + public void putResource(Object key, Object value) + throws IllegalStateException { + delegate.putResource(key, value); + } + +}