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();
    }
}