Hi, I've tried a patch catching Throwable and it works as expected, with CDI closing the transaction and Eclipselink not getting bugged. I think this should be part of tomee. Is there any reason for this Interceptor for not catching StackOverflowError or an AssertionError and leaving the underlying Transaction in an inconsistent state?
I can provide a PR if you want, the patch is really straightforward, just change Exception for Throwable and rethrow as an exception to make the compiler happy: /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with * this work for additional information regarding copyright ownership. * The ASF licenses this file to You under the Apache License, Version 2.0 * (the "License"); you may not use this file except in compliance with * the License. You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ package org.apache.openejb.cdi.transactional; import org.apache.openejb.ApplicationException; import org.apache.openejb.OpenEJB; import org.apache.openejb.SystemException; import org.apache.openejb.core.CoreUserTransaction; import org.apache.openejb.core.transaction.TransactionPolicy; import org.apache.openejb.core.transaction.TransactionRolledbackException; import org.apache.openejb.loader.SystemInstance; import javax.enterprise.inject.spi.AnnotatedMethod; import javax.enterprise.inject.spi.AnnotatedType; import javax.enterprise.inject.spi.CDI; import javax.interceptor.InvocationContext; import javax.transaction.RollbackException; import javax.transaction.TransactionManager; import javax.transaction.TransactionRequiredException; import javax.transaction.Transactional; import javax.transaction.TransactionalException; import java.io.Serializable; import java.lang.reflect.Method; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; import java.util.logging.Logger; import static java.util.Arrays.asList; public abstract class InterceptorBase implements Serializable { private static final IllegalStateException ILLEGAL_STATE_EXCEPTION = new IllegalStateException("Can't use UserTransaction from @Transactional call"); private static final boolean HANDLE_EXCEPTION_ONLY_FOR_CLIENT = SystemInstance.get().getOptions().get("openejb.cdi.jta.exception.client-only", false); private transient volatile ConcurrentMap<Method, Boolean> rollback = new ConcurrentHashMap<>(); protected Object intercept(final InvocationContext ic) throws Exception { TransactionPolicy policy = null; final boolean forbidsUt = doesForbidUtUsage(); final RuntimeException oldEx; final IllegalStateException illegalStateException; if (forbidsUt) { illegalStateException = ILLEGAL_STATE_EXCEPTION; oldEx = CoreUserTransaction.error(illegalStateException); } else { illegalStateException = null; oldEx = null; } try { policy = getPolicy(); final Object proceed = ic.proceed(); policy.commit(); // force commit there to ensure we can catch synchro exceptions return proceed; } catch (final Throwable e) { if (illegalStateException == e) { throw e; } Throwable error = unwrap(e); if (error != null && (!HANDLE_EXCEPTION_ONLY_FOR_CLIENT || policy.isNewTransaction())) { final Method method = ic.getMethod(); if (rollback == null) { synchronized (this) { if (rollback == null) { rollback = new ConcurrentHashMap<>(); } } } Boolean doRollback = rollback.get(method); if (doRollback != null) { if (doRollback && policy != null && policy.isTransactionActive()) { policy.setRollbackOnly(); } } else { // computed lazily but we could cache it later for sure if that's really a normal case final AnnotatedType<?> annotatedType = CDI.current().getBeanManager().createAnnotatedType(method.getDeclaringClass()); Transactional tx = null; for (final AnnotatedMethod<?> m : annotatedType.getMethods()) { if (method.equals(m.getJavaMember())) { tx = m.getAnnotation(Transactional.class); break; } } if (tx == null) { tx = annotatedType.getAnnotation(Transactional.class); } if (tx != null) { doRollback = new ExceptionPriotiryRules(tx.rollbackOn(), tx.dontRollbackOn()).accept(error, method.getExceptionTypes()); rollback.putIfAbsent(method, doRollback); if (doRollback && policy != null && policy.isTransactionActive()) { policy.setRollbackOnly(); } } } } if (policy != null) { try { policy.commit(); } catch (final Exception ex) { // no-op: swallow to keep the right exception final Logger logger = Logger.getLogger(getClass().getName()); if (logger.isLoggable(Level.FINE)) { logger.fine("Swallowing: " + ex.getMessage()); } } } if (error == null || TransactionRequiredException.class.isInstance(error)) { throw new TransactionalException(e.getMessage(), error); } throw rethrow(error); } finally { if (forbidsUt) { CoreUserTransaction.resetError(oldEx); } } } private static <T extends Throwable> T rethrow(Throwable exception) throws T { throw (T) exception; } private Throwable unwrap(Throwable e) { Throwable error = e; while (error != null && (ApplicationException.class.isInstance(error) || SystemException.class.isInstance(error) || TransactionRolledbackException.class.isInstance(error))) { final Throwable cause = error.getCause(); if (cause == error) { break; } error = Exception.class.isInstance(cause) ? Exception.class.cast(cause) : null; } if (RollbackException.class.isInstance(error) && Exception.class.isInstance(error.getCause())) { error = Exception.class.cast(error.getCause()); } return error; } protected boolean doesForbidUtUsage() { return true; } protected abstract TransactionPolicy getPolicy() throws SystemException, ApplicationException; protected static TransactionManager getTransactionManager() { return OpenEJB.getTransactionManager(); } protected static final class ExceptionPriotiryRules { private final Class<?>[] includes; private final Class<?>[] excludes; protected ExceptionPriotiryRules(final Class<?>[] includes, final Class<?>[] excludes) { this.includes = includes; this.excludes = excludes; } public boolean accept(final Throwable e, final Class<?>[] exceptionTypes) { if (e == null) { return false; } final int includeScore = contains(includes, e); final int excludeScore = contains(excludes, e); if (excludeScore < 0) { return includeScore >= 0 || isNotChecked(e, exceptionTypes); } return includeScore - excludeScore > 0; } private static int contains(final Class<?>[] list, final Throwable e) { int score = -1; for (final Class<?> clazz : list) { if (clazz.isInstance(e)) { final int thisScore = score(clazz, e.getClass()); if (score < 0) { score = thisScore; } else { score = Math.min(thisScore, score); } } } return score; } private static int score(final Class<?> config, final Class<?> ex) { int score = 0; Class<?> current = ex; while (current != null && !current.equals(config)) { score++; current = current.getSuperclass(); } return score; } private static boolean isNotChecked(final Throwable e, final Class<?>[] exceptionTypes) { return RuntimeException.class.isInstance(e) && (exceptionTypes.length == 0 || !asList(exceptionTypes).contains(e.getClass())); } } } On Wed, Nov 30, 2022 at 6:07 PM Vicente Rossello <cocorosse...@gmail.com> wrote: > Hi, > > We are using tomee with JPA and eclipselink implementation, in latest > tomee 8.0. > > Whenever we have a StackOverflowError (very rare case, fortunately :) ), > the application server is in an inconsistent state, with eclipselink > throwing some random errors, we have to shut the instance down. > > I think it is because the transaction is never closed. I can see that the > class org.apache.openejb.cdi.transactional.InterceptorBase only catches > Exception. > > ... > > try { > policy = getPolicy(); > final Object proceed = ic.proceed(); > policy.commit(); // force commit there to ensure we can catch synchro > exceptions > return proceed; > } catch (final Exception e) { > if (illegalStateException == e) { > throw e; > } > > .... > > > Shouldn't it be Throwable? or at least catch some Error specific classes? > WDYT? > > > Thanks, > Vicente >
