Blog

Friday 5 July 2013

5 lines to kill seam3 based production

If you are using seam3 transactions in your project then it takes only 5 lines of code to kill it.
Learn how to defend yourself.
You may also experience this with following message in logs:
javax.transaction.NotSupportedException: BaseTransaction.checkTransactionState - ARJUNA016051: thread is already associated with a transaction!

If you'd look into stactrace you would see that it's org.jboss.seam.transaction.TransactionServletListener who's trying to begin transaction without checking if transaction is already associated with current thread.
Well, looking at the code it seems that it's not possible that TransactionServletListener could start transaction at the beginning of request and not finish it after the request.
After long hours of debugging and testing it turned out that sometimes the transaction may time out. In such case the transaction reaper rolls it back. The problem is however that it does not disassociate transaction from thread it was started in.
So we need to modify our TransactionServletListener to call rollback even on transaction in "ROLLED_BACK" state:
int status = tx.getStatus();

if (status == Status.STATUS_MARKED_ROLLBACK || status == Status.STATUS_ROLLEDBACK) {
    LOG.warn("Transaction was already started before the listener and is marked for rollback or rolled back from other thread,"
                        + " so doing rollback to disassociate it with current thread");
    tx.rollback();
}

Ok, this solves the problem, but how can transaction timeout for a regular request?
  • client is downloading large file slowly
  • cient opens connection and doesn't close it within transaction timeout
  • TransactionServletListener does not play nice with Push servlets since they keep connection open
Let's assume your transaction timeout is set to 10 seconds. You only need following lines of code to reproduce this issue:
final String url = "http://localhost:8080/sample/anyFile.zip";
final InputStream inputStream = new URL(url).openStream();
synchronized (this) {
     Thread.sleep(15000);
}

inputStream.close();

Updated 2013/07/19

Many of you ask for the implementation of the listener. Here it is:
package pl.itcrowd.seam3.transaction;

import org.apache.commons.lang.StringUtils;
import org.jboss.seam.transaction.DefaultTransaction;
import org.jboss.seam.transaction.SeamTransaction;
import org.jboss.solder.exception.control.ExceptionToCatch;
import org.jboss.solder.logging.Logger;

import javax.enterprise.event.Event;
import javax.inject.Inject;
import javax.servlet.ServletRequestEvent;
import javax.servlet.ServletRequestListener;
import javax.servlet.annotation.WebListener;
import javax.servlet.http.HttpServletRequest;
import javax.transaction.HeuristicMixedException;
import javax.transaction.HeuristicRollbackException;
import javax.transaction.NotSupportedException;
import javax.transaction.RollbackException;
import javax.transaction.Status;
import javax.transaction.SystemException;
import java.util.ArrayList;
import java.util.List;
import java.util.StringTokenizer;

/**
 * Listener to begin / commit / rollback a transaction around each request.
 * <p/>
 * Configure which paths are included/excluded with context params in web.xml:
 * <p/>
 * <pre>
 * <context-param>
 *  <param-name>pl.itcrowd.seam.transaction.ConfigurableTransactionServletListener.includes</param-name>
 *  <param-value>.*\.jsf$,/photoServlet</param-value>
 * &lt/context-param>
 *  </pre>
 *
 * @author <a href="http://community.jboss.org/people/blabno">Bernard Labno</a>
 */
@WebListener
public class ConfigurableTransactionServletListener implements ServletRequestListener {

    /**
     * context-param to disable the listener.
     */
    public static final String DISABLE_LISTENER_PARAM = "pl.itcrowd.seam3.transaction.disableListener";

    private static final String EXCLUDES_KEY = "pl.itcrowd.seam3.transaction.excludes";

    private static final String INCLUDES_KEY = "pl.itcrowd.seam3.transaction.includes";

    private static final Logger LOG = Logger.getLogger(ConfigurableTransactionServletListener.class);

    @Inject
    Event<ExceptionToCatch> txException;

    @SuppressWarnings("CdiInjectionPointsInspection")
    @Inject
    @DefaultTransaction
    private SeamTransaction tx;

    @Override
    public void requestDestroyed(ServletRequestEvent sre)
    {
        final String listenerDisabledParam = sre.getServletContext().getInitParameter(DISABLE_LISTENER_PARAM);
        if (listenerDisabledParam != null && "true".equals(listenerDisabledParam.trim().toLowerCase())) {
            return;
        }
        final HttpServletRequest request = (HttpServletRequest) sre.getServletRequest();
        final boolean include = matches(request, getIncludes(sre));
        final boolean exclude = matches(request, getExcludes(sre));
        LOG.debugv("Request destroyed: {3} | include:{0}, exclude:{1}, url:{2}", include, exclude, request.getServletPath(), request.getRequestURI());
        if (include && !exclude) {
            try {
                switch (this.tx.getStatus()) {
                    case Status.STATUS_ACTIVE:
                        LOG.debugf("Committing a transaction for request %s", request.getRequestURI());
                        tx.commit();
                        break;
                    case Status.STATUS_MARKED_ROLLBACK:
                    case Status.STATUS_PREPARED:
                    case Status.STATUS_PREPARING:
                        LOG.debugf("Rolling back a transaction for request %s", request.getRequestURI());
                        tx.rollback();
                        break;
                    case Status.STATUS_COMMITTED:
                    case Status.STATUS_COMMITTING:
                    case Status.STATUS_ROLLING_BACK:
                    case Status.STATUS_UNKNOWN:
                    case Status.STATUS_ROLLEDBACK:
                    case Status.STATUS_NO_TRANSACTION:
                        break;
                }
            } catch (SystemException e) {
                LOG.warn("Error rolling back the transaction", e);
                this.txException.fire(new ExceptionToCatch(e));
            } catch (HeuristicRollbackException e) {
                LOG.warn("Error committing the transaction", e);
                this.txException.fire(new ExceptionToCatch(e));
            } catch (RollbackException e) {
                LOG.warn("Error committing the transaction", e);
                this.txException.fire(new ExceptionToCatch(e));
            } catch (HeuristicMixedException e) {
                LOG.warn("Error committing the transaction", e);
                this.txException.fire(new ExceptionToCatch(e));
            }
        }
    }

    @Override
    public void requestInitialized(ServletRequestEvent sre)
    {
        final String listenerDisabledParam = sre.getServletContext().getInitParameter(DISABLE_LISTENER_PARAM);
        if (listenerDisabledParam != null && "true".equals(listenerDisabledParam.trim().toLowerCase())) {
            return;
        }
        final HttpServletRequest request = (HttpServletRequest) sre.getServletRequest();
        final boolean include = matches(request, getIncludes(sre));
        final boolean exclude = matches(request, getExcludes(sre));
        LOG.debugv("Request initialized: {3} | include:{0}, exclude:{1}, url:{2}", include, exclude, request.getServletPath(), request.getRequestURI());
        if (include && !exclude) {
            try {
                int status = tx.getStatus();
                if (status == Status.STATUS_MARKED_ROLLBACK || status == Status.STATUS_ROLLEDBACK) {
                    LOG.warn("Transaction was already started before the listener and is marked for rollback or rolled back from other thread,"
                        + " so doing rollback to disassociate it with current thread");
                    tx.rollback();
                } else if (status != Status.STATUS_NO_TRANSACTION) {
                    LOG.warnv("Transaction was already started before the listener. Transaction status: {0}", status);
                }
                status = tx.getStatus();
                if (status == Status.STATUS_ACTIVE) {
                    LOG.warn("Transaction was already started before the listener");
                } else {
                    LOG.debugf("Beginning transaction for request %s", request.getRequestURI());
                    this.tx.begin();
                }
            } catch (SystemException se) {
                LOG.warn("Error starting the transaction, or checking status", se);
                this.txException.fire(new ExceptionToCatch(se));
            } catch (NotSupportedException e) {
                LOG.warn("Error starting the transaction", e);
                this.txException.fire(new ExceptionToCatch(e));
            }
        }
    }

    private List<String> getExcludes(ServletRequestEvent sre)
    {
        return getPatterns(sre, EXCLUDES_KEY);
    }

    private List<String> getIncludes(ServletRequestEvent sre)
    {
        return getPatterns(sre, INCLUDES_KEY);
    }

    private List<String> getPatterns(ServletRequestEvent sre, String key)
    {
        final Object attribute = sre.getServletContext().getAttribute(key);
        final List<String> patterns;
        if (null == attribute || !(attribute instanceof List)) {
            final String initParameter = sre.getServletContext().getInitParameter(key);
            patterns = new ArrayList<String>();
            if (!StringUtils.isBlank(initParameter)) {
                final StringTokenizer tokenizer = new StringTokenizer(initParameter, ",");
                while (tokenizer.hasMoreElements()) {
                    patterns.add(tokenizer.nextToken());
                }
            }
            sre.getServletContext().setAttribute(key, patterns);
        } else {
            //noinspection unchecked
            patterns = (List<String>) attribute;
        }
        return patterns;
    }

    private boolean matches(HttpServletRequest servletRequest, List<String> patterns)
    {
        final String servletPath = servletRequest.getServletPath();
        for (String pattern : patterns) {
            if (servletPath.matches(pattern)) {
                return true;
            }
        }
        return patterns.isEmpty();
    }
}

7 comments:

  1. Hi Bernard,

    Its a very nice Post. The above post seems to be quite true about the given seam transaction situations. But what about automatic transaction management performing by container itself?. I have been using CMT Transaction, which is basically managed by EJB container automatically, in that case how can we solve the problem? I am still facing the same kind of problem and I am using EJB 3.1, JPA with Hibernate and Prime Faces 3.3.1. Can you please give us some ideas on this matter?

    ReplyDelete
    Replies
    1. It doesn't matter if you're using CMT on your EJB. If Seam listener starts the transaction then by default ejb will reuse that transaction.

      Delete
  2. While working on a fix for https://issues.jboss.org/browse/WFLY-1346 (avoid "transaction is not active" error by having CMTTxInterceptor.endTransaction check transaction status), I also saw a similar problem where I needed to clear the (CMT) threads active transaction. If others are still having problems with the transaction not being cleared from the thread, please try WildFly 8.0.0.Alpha2 or newer, to see if that helps at all. Another tip is that you might want to enable TRACE logging for Arjuna (see https://docs.jboss.org/author/display/WFLY8/JPA+Reference+Guide#JPAReferenceGuide-Troubleshooting for example).

    ReplyDelete
  3. Hello Bernard,

    I am facing this same problem in my application. What do you mean by 'to modify our TransactionServletListener'?. Do you extend the org.jboss.seam.transaction.TransactionServletListener? Overriding the one from Seam?

    Thank you

    ReplyDelete
    Replies
    1. Hi, I've updated the article with the source code of the listener.

      Delete
  4. great work, this error confuse me a long time, I have upgraded my application and I will see jboss log.

    ReplyDelete
  5. Labno, I have a question, do you think it will add system load if we use the seam's default transactionListener? I debug it and found every resource(js,css,image) request will begin a transaction, is it right? I use your configurableTransactionListener instead, and configure these request into excludes list. do you think so?

    ReplyDelete