You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@tomee.apache.org by Vicente Rossello <co...@gmail.com> on 2022/11/30 17:07:38 UTC

Transactional CDI

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

Re: Transactional CDI

Posted by Vicente Rossello <co...@gmail.com>.
But system vs application exceptions is about whether the container has to
commit or rollback the transaction. In this error case the transaction is
not handled at all.

I can see that in tomee it's handled for EJB. You can see that
EjbTransactionUtil.afterInvoke is always invoked in the finally clause, not
in the catch clause. The transaction is handled even if there is an Error.

Shouldn't it be the same?



On Mon, Dec 5, 2022 at 6:05 PM Jean-Louis Monteiro <jl...@tomitribe.com>
wrote:

> 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 <co...@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 <cocorossello@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
> > >
> >
>

Re: Transactional CDI

Posted by Jean-Louis Monteiro <jl...@tomitribe.com>.
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 <co...@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 <co...@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
> >
>

Re: Transactional CDI

Posted by Vicente Rossello <co...@gmail.com>.
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 <co...@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
>