479256: Transactional Resources Leak in ServerJob
Change-Id: If489065b0439a910cc2efd400900e6d7f20d4575
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=479256
Reviewed-on: https://git.eclipse.org/r/57652
Reviewed-by: Andi Bur <andi.bur@gmail.com>
Tested-by: Andi Bur <andi.bur@gmail.com>
diff --git a/org.eclipse.scout.rt.server.test/src/org/eclipse/scout/rt/server/ServerJobTest.java b/org.eclipse.scout.rt.server.test/src/org/eclipse/scout/rt/server/ServerJobTest.java
index e56b7ab..8d10503 100644
--- a/org.eclipse.scout.rt.server.test/src/org/eclipse/scout/rt/server/ServerJobTest.java
+++ b/org.eclipse.scout.rt.server.test/src/org/eclipse/scout/rt/server/ServerJobTest.java
@@ -11,7 +11,10 @@
package org.eclipse.scout.rt.server;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+import static org.mockito.Matchers.any;
import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -25,9 +28,17 @@
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.core.runtime.Status;
+import org.eclipse.scout.commons.exception.ProcessingException;
+import org.eclipse.scout.rt.server.internal.Activator;
import org.eclipse.scout.rt.server.transaction.ITransaction;
+import org.eclipse.scout.rt.server.transaction.ITransactionMember;
+import org.eclipse.scout.rt.shared.services.common.exceptionhandler.IExceptionHandlerService;
import org.eclipse.scout.rt.testing.shared.HandlerAdapter;
+import org.eclipse.scout.rt.testing.shared.TestingUtility;
import org.junit.Test;
+import org.mockito.InOrder;
+import org.mockito.Mockito;
+import org.osgi.framework.ServiceRegistration;
/**
* Tests for {@link ServerJob}
@@ -36,6 +47,7 @@
*/
public class ServerJobTest {
+ private static final ProcessingException EXPECTED_PROCESSING_EXCEPTION = new ProcessingException("Expected exception");
private static final RuntimeException m_rollBackException = new RuntimeException("Exception in transaction.rollBack()!");
private static final Throwable[] m_transactionFailures = new Throwable[2];
static {
@@ -54,6 +66,51 @@
assertEquals("Could not find transactionFailure1 in the Logs!", true, interceptor.hasExceptionLogged(m_transactionFailures[1]));
}
+ @Test
+ public void testRollbackAndRelease() throws Exception {
+ doTestRollbackAndRelease();
+ }
+
+ @Test
+ public void testRollbackAndReleaseUsingCustomExceptionHandlerService() throws Exception {
+ final IExceptionHandlerService exceptionHandlerService = mock(IExceptionHandlerService.class);
+ Mockito.doThrow(new RuntimeException("translated exception")).when(exceptionHandlerService).handleException(any(ProcessingException.class));
+ List<ServiceRegistration> testServices = TestingUtility.registerServices(Activator.getDefault().getBundle(), 1000, exceptionHandlerService);
+
+ try {
+ doTestRollbackAndRelease();
+ }
+ finally {
+ TestingUtility.unregisterServices(testServices);
+ }
+ }
+
+ protected void doTestRollbackAndRelease() {
+ final ITransactionMember txnMember = mock(ITransactionMember.class);
+ when(txnMember.needsCommit()).thenReturn(true);
+
+ final ServerJob job = new ServerJob("test rollback and release job", mock(IServerSession.class)) {
+ @Override
+ protected IStatus runTransaction(IProgressMonitor monitor) throws Exception {
+ ITransaction txn = ThreadContext.getTransaction();
+ txn.registerMember(txnMember);
+ throw EXPECTED_PROCESSING_EXCEPTION;
+ }
+ };
+
+ job.runNow(new NullProgressMonitor());
+ try {
+ job.throwOnError();
+ }
+ catch (ProcessingException e) {
+ assertSame(EXPECTED_PROCESSING_EXCEPTION, e);
+ }
+
+ InOrder order = inOrder(txnMember);
+ order.verify(txnMember).rollback();
+ order.verify(txnMember).release();
+ }
+
private P_LogHandler installLogInterceptor() {
Logger logger = LogManager.getLogManager().getLogger(ServerJob.class.getName());
P_LogHandler handler = new P_LogHandler();
@@ -80,7 +137,6 @@
doThrow(m_rollBackException).when(transaction).rollback(); //Throw unchecked exception here!
return transaction;
}
-
}
private class P_LogHandler extends HandlerAdapter {
@@ -101,5 +157,4 @@
return false;
}
}
-
}
diff --git a/org.eclipse.scout.rt.server/src/org/eclipse/scout/rt/server/ServerJob.java b/org.eclipse.scout.rt.server/src/org/eclipse/scout/rt/server/ServerJob.java
index 2861d1e..2bf1765 100644
--- a/org.eclipse.scout.rt.server/src/org/eclipse/scout/rt/server/ServerJob.java
+++ b/org.eclipse.scout.rt.server/src/org/eclipse/scout/rt/server/ServerJob.java
@@ -251,25 +251,31 @@
finally {
ActiveTransactionRegistry.unregister(transaction);
if (transaction.hasFailures()) {
- try {
- IExceptionHandlerService exceptionHandlerService = SERVICES.getService(IExceptionHandlerService.class);
- for (Throwable transactionFailure : transaction.getFailures()) {
- if (transactionFailure instanceof ProcessingException) {
+ IExceptionHandlerService exceptionHandlerService = SERVICES.getService(IExceptionHandlerService.class);
+ for (Throwable transactionFailure : transaction.getFailures()) {
+ if (transactionFailure instanceof ProcessingException) {
+ try {
exceptionHandlerService.handleException((ProcessingException) transactionFailure);
}
- else {
- LOG.error("Transaction had failure.", transactionFailure);
+ catch (Throwable t) {
+ if (LOG.isDebugEnabled()) {
+ LOG.warn("Handling transaction failure throwed another exception", t);
+ }
+ else {
+ LOG.warn("Handling transaction failure throwed another exception of type '{}'", t.getClass().getName());
+ }
}
}
+ else {
+ LOG.error("Transaction had failure.", transactionFailure);
+ }
}
- finally {
- // xa rollback
- try {
- transaction.rollback();
- }
- catch (Throwable t) {
- LOG.error("Transaction rollback failed with exception.", t);
- }
+ // xa rollback
+ try {
+ transaction.rollback();
+ }
+ catch (Throwable t) {
+ LOG.error("Transaction rollback failed with exception.", t);
}
}
else {
