I think it's related to the definition of System versus Application exception per specification. I would need to check again as I don't recall.
-- Jean-Louis Monteiro http://twitter.com/jlouismonteiro http://www.tomitribe.com On Fri, Dec 2, 2022 at 11:58 PM Vicente Rossello <cocorosse...@gmail.com> wrote: > 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 > > >
