From 5eb0c4fe8b87a2ce7480e1d1d40ba97021e01c01 Mon Sep 17 00:00:00 2001 From: Francisco Javier Tirado Sarti <65240126+fjtirado@users.noreply.github.com> Date: Mon, 6 Nov 2023 17:23:41 +0100 Subject: [PATCH 1/2] Merge pull request #11 from kiegroup/JBPM-10187 [JBPM-10187] Sorting resources to avoid deadlock --- .../api/TransactionManagerHelper.java | 17 ++++++++++------- .../persistence/jta/JtaTransactionManager.java | 4 ++-- .../drools/persistence/PersistableRunner.java | 14 ++++++++++---- .../persistence/jpa/JpaPersistenceContext.java | 4 ++-- .../jpa/OptimisticLockRetryInterceptor.java | 2 +- .../JPAPlaceholderResolverStrategy.java | 7 +++++++ 6 files changed, 32 insertions(+), 16 deletions(-) diff --git a/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java b/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java index adf917317dc..cdd6e6adf3a 100644 --- a/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java +++ b/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java @@ -16,13 +16,15 @@ package org.drools.persistence.api; import java.util.Collections; +import java.util.Comparator; import java.util.LinkedHashSet; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; public class TransactionManagerHelper { private static final String APP_UPDETEABLE_RESOURCE = "app-updateable-resource"; - private static final String CMD_UPDETEABLE_RESOURCE = "cmd-updateable-resource"; public static void registerTransactionSyncInContainer(TransactionManager txm, OrderedTransactionSynchronization synchronization) { TransactionSynchronizationContainer container = (TransactionSynchronizationContainer)txm.getResource(TransactionSynchronizationContainer.RESOURCE_KEY); @@ -41,7 +43,7 @@ public static void addToUpdatableSet(TransactionManager txm, Transformable trans } Set toBeUpdated = (Set) txm.getResource(APP_UPDETEABLE_RESOURCE); if (toBeUpdated == null) { - toBeUpdated = new LinkedHashSet(); + toBeUpdated = new LinkedHashSet<>(); txm.putResource(APP_UPDETEABLE_RESOURCE, toBeUpdated); } toBeUpdated.add(transformable); @@ -58,11 +60,12 @@ public static void removeFromUpdatableSet(TransactionManager txm, Transformable @SuppressWarnings("unchecked") public static Set getUpdateableSet(TransactionManager txm) { - Set toBeUpdated = (Set) txm.getResource(APP_UPDETEABLE_RESOURCE); - if (toBeUpdated == null) { - return Collections.emptySet(); + Set result = (Set) txm.getResource(APP_UPDETEABLE_RESOURCE); + if (result != null) { + SortedSet sorted = new TreeSet<>(Comparator.comparing(o -> o.getClass().getSimpleName())); + sorted.addAll(result); + return sorted; } - - return new LinkedHashSet(toBeUpdated); + return Collections.emptySet(); } } diff --git a/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/jta/JtaTransactionManager.java b/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/jta/JtaTransactionManager.java index 9c5f3c6ae30..3950b6922df 100644 --- a/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/jta/JtaTransactionManager.java +++ b/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/jta/JtaTransactionManager.java @@ -117,9 +117,9 @@ protected javax.transaction.TransactionManager findTransactionManager(UserTransa jndiName ); return tm; } catch ( NamingException ex ) { - logger.debug( "No JTA TransactionManager found at fallback JNDI location [{}]", + logger.debug( "No JTA TransactionManager found at fallback JNDI location [{}]. Exception message {}", jndiName, - ex); + ex.getMessage()); } } diff --git a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/PersistableRunner.java b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/PersistableRunner.java index 0e7d302f7ff..73be05b2c72 100644 --- a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/PersistableRunner.java +++ b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/PersistableRunner.java @@ -151,7 +151,7 @@ protected void initNewKnowledgeSession(KieBase kbase, KieSessionConfiguration co ((InternalKnowledgeRuntime) this.ksession).setEndOperationListener( new EndOperationListenerImpl(this.txm, this.sessionInfo ) ); - this.runner = new TransactionInterceptor(); + this.runner = new TransactionInterceptor(sessionInfo); TimerJobFactoryManager timerJobFactoryManager = ((InternalKnowledgeRuntime) ksession ).getTimerService().getTimerJobFactoryManager(); if (timerJobFactoryManager instanceof CommandServiceTimerJobFactoryManager) { @@ -258,7 +258,7 @@ protected void initExistingKnowledgeSession(Long sessionId, kruntime.setIdentifier( this.sessionInfo.getId() ); kruntime.setEndOperationListener( new EndOperationListenerImpl( this.txm, this.sessionInfo ) ); - this.runner = new TransactionInterceptor(); + this.runner = new TransactionInterceptor(sessionInfo); // apply interceptors Iterator iterator = this.interceptors.descendingIterator(); while (iterator.hasNext()) { @@ -504,6 +504,7 @@ public void afterCompletion(int status) { this.service.jpm.endCommandScopedEntityManager(); KieSession ksession = this.service.ksession; + logger.debug("Cleaning up session {} information", ksession != null ? ksession.getIdentifier() : null); // clean up cached process and work item instances if ( ksession != null ) { InternalProcessRuntime internalProcessRuntime = ((InternalWorkingMemory) ksession).internalGetProcessRuntime(); @@ -513,8 +514,11 @@ public void afterCompletion(int status) { } internalProcessRuntime.clearProcessInstances(); + logger.debug("Cached process instances after clean up {}",internalProcessRuntime.getProcessInstances()); } + ((JPAWorkItemManager) ksession.getWorkItemManager()).clearWorkItems(); + } if (status != TransactionManager.STATUS_COMMITTED) { this.service.jpm.resetApplicationScopedPersistenceContext(); @@ -567,14 +571,16 @@ private void registerUpdateSync() { private class TransactionInterceptor extends AbstractInterceptor { + private SessionInfo sessionInfo; - - public TransactionInterceptor() { + public TransactionInterceptor(SessionInfo sessionInfo) { + this.sessionInfo = sessionInfo; setNext(new PseudoClockRunner()); } @Override public RequestContext execute( Executable executable, RequestContext context ) { + if ( !( (InternalExecutable) executable ).canRunInTransaction() ) { executeNext(executable, context); if (((InternalExecutable) executable ).requiresDispose()) { diff --git a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/JpaPersistenceContext.java b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/JpaPersistenceContext.java index a84e1cfd937..db70da750d2 100644 --- a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/JpaPersistenceContext.java +++ b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/JpaPersistenceContext.java @@ -32,7 +32,7 @@ public class JpaPersistenceContext implements PersistenceContext { - private static Logger logger = LoggerFactory.getLogger(JpaPersistenceContext.class); + protected static Logger logger = LoggerFactory.getLogger(JpaPersistenceContext.class); private EntityManager em; protected final boolean isJTA; @@ -68,7 +68,7 @@ public PersistentSession persist(PersistentSession entity) { } public PersistentSession findSession(Long id) { - + logger.trace("Reading session info {}",id); SessionInfo sessionInfo = null; if( this.pessimisticLocking ) { sessionInfo = this.em.find( SessionInfo.class, id, lockMode ); diff --git a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/OptimisticLockRetryInterceptor.java b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/OptimisticLockRetryInterceptor.java index 9ba5fadcb38..e318dba30cf 100644 --- a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/OptimisticLockRetryInterceptor.java +++ b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/OptimisticLockRetryInterceptor.java @@ -94,7 +94,7 @@ protected RequestContext internalExecute( Executable executable, RequestContext RuntimeException originException = null; while (true) { - if (attempt > 1) { + if (attempt > 0) { logger.trace("retrying (attempt {})...", attempt); } try { diff --git a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/marshaller/JPAPlaceholderResolverStrategy.java b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/marshaller/JPAPlaceholderResolverStrategy.java index 0c637ec2ea2..16840840da4 100644 --- a/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/marshaller/JPAPlaceholderResolverStrategy.java +++ b/drools-persistence/drools-persistence-jpa/src/main/java/org/drools/persistence/jpa/marshaller/JPAPlaceholderResolverStrategy.java @@ -203,6 +203,7 @@ private boolean isEntity(Object o){ public void onStart(TransactionManager txm) { if (persister.get() == null) { EntityManager em = emf.createEntityManager(); + log.trace ("Created EM {} for JPAPlaceHolder {}:{}",em, this, name); persister.set(new EntityPersister(em)); } } @@ -210,6 +211,11 @@ public void onStart(TransactionManager txm) { @Override public void onEnd(TransactionManager txm) { EntityPersister em = persister.get(); + if (em == null) { + log.warn ("EM is null for {}:{} and status {}", this, name, txm.getStatus()); + return; + } + log.trace ("Executing onEnd for {}:{} with status {}", this, name, txm.getStatus()); if(txm.getStatus() == TransactionManager.STATUS_ROLLEDBACK) { // this is pretty much of a hack but for avoiding issues when rolling back we need to set to null // the primary key of the entities (simple types) @@ -239,6 +245,7 @@ public void onEnd(TransactionManager txm) { }); } if (em != null) { + log.trace ("Closing EM {} for JPAPlaceHolder {}:{}",em.getEntityManager(), this, name); em.close(); persister.set(null); } From ec9ee4a3a665c413fde294c728cba3a758df0886 Mon Sep 17 00:00:00 2001 From: Francisco Javier Tirado Sarti <65240126+fjtirado@users.noreply.github.com> Date: Wed, 8 Nov 2023 13:44:28 +0100 Subject: [PATCH 2/2] Update TransactionManagerHelper.java --- .../drools/persistence/api/TransactionManagerHelper.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java b/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java index cdd6e6adf3a..6d5f8abf967 100644 --- a/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java +++ b/drools-persistence/drools-persistence-api/src/main/java/org/drools/persistence/api/TransactionManagerHelper.java @@ -16,7 +16,6 @@ package org.drools.persistence.api; import java.util.Collections; -import java.util.Comparator; import java.util.LinkedHashSet; import java.util.Set; import java.util.SortedSet; @@ -62,7 +61,10 @@ public static void removeFromUpdatableSet(TransactionManager txm, Transformable public static Set getUpdateableSet(TransactionManager txm) { Set result = (Set) txm.getResource(APP_UPDETEABLE_RESOURCE); if (result != null) { - SortedSet sorted = new TreeSet<>(Comparator.comparing(o -> o.getClass().getSimpleName())); + SortedSet sorted = new TreeSet<>((o1, o2) -> { + int compared = o1.getClass().getSimpleName().compareTo(o2.getClass().getSimpleName()); + return compared == 0 ? 1 : compared; + }); sorted.addAll(result); return sorted; }