Seam Faces-Transaction integration

[rcd]

Seam Faces has the TransactionPhaseListener, which according to the Seam Faces homepage, is supposed to start a transaction before the RestoreView phase, then commit it before the RenderResponse phase. I have some code that is being run during the InvokeApplication phase. If I inject the SeamTransaction and check its status, it returns 6 (STATUS_NO_TRANSACTION).

After looking at the source of TransactionPhaseListener, I think this is a bug. The beforePhase() method calls handleTransactionsBeforePhase(), which in turn only calls begin() to start the transaction if the current phase is RenderResponse:

    public void handleTransactionsBeforePhase(final PhaseEvent event) {
        PhaseId phaseId = event.getPhaseId();
        if (seamManagedTransactionStatus(phaseId)) {
            if (phaseId == RENDER_RESPONSE) {
                persistenceContexts.beforeRender();
                begin(phaseId);
            }
        }
    }

It looks like the call to begin() should be outside that inner if statement, like so:

    public void handleTransactionsBeforePhase(final PhaseEvent event) {
        PhaseId phaseId = event.getPhaseId();
        if (seamManagedTransactionStatus(phaseId)) {
            if (phaseId == RENDER_RESPONSE) {
                persistenceContexts.beforeRender();
            }
            begin(phaseId);
        }
    }

[lightguard]

It's possible that's a bug. The whole transaction stuff has been riddled with bugs. What server are you using?

[rcd]

Jason Porter wrote on Jan 18, 2012 13:44:


It's possible that's a bug. The whole transaction stuff has been riddled with bugs. What server are you using?

JBoss 7.0.2.Final with Seam 3.1.0.Final.

[lightguard]

Does this change seem to have any other affects or create transactions where they're not needed in other phases?

[rcd]

I have no idea; I haven't actually tried it. Although now that I'm thinking about it, calling begin() at the start of EVERY phase is wrong too... it should only start before RestoreView and RenderResponse according to the documentation.

As to the transaction stuff has been riddled with bugs: that would explain the duplicate transaction that seemed to be created when I tried to work around the original problem by injecting SeamTransaction into my bean and having it start the transaction itself before calling my DAO. The bean makes several calls to the DAO and I need them all to happen in the same transaction. This seemed to work, since I didn't see beginning JTA transaction between the method calls when I added some additional logging, although it's hard to tell since different instances of SeamTransaction got injected in different places. I then saw that the transaction got committed after the InvokeApplication phase, as it is supposed to be, but a few minutes later, a bunch of warnings appeared saying that a database transaction had timed out and been rolled back.

[lightguard]

Yep, the transaction code is split between that phase listener and also a servlet requset listener in Seam Transaction. You can check the docs for more info and also how to disable it.

Seems like you've been doing quite a bit of digging Richard, I'd like to see if you have a better solution, or a bug fix. Would you be willing to take that on?

[rcd]

I'm willing to take a crack at it, but it may be a couple days before I have time.

[lightguard]

Not a problem at all. Thank you very much for the help and contributions!

[blabno]

Richard and Jason,

Allow me to add another seam transaction aspect to this thread.

Let's imagine we have 2 components A and B.
Component A can throw NoResultException (its RuntimeException) so by default ExceptionUtil says yes, roll it back, but that component is used from within component B that is aware of that exception and handles it gracefully.
But anyway the transaction gets rolled back by interceptor (around method invocation on component A) and later in the same request i get javax.persistence.TransactionRequiredException: No active JTA transaction on joinTransaction call.

For JSF apps shouldn't transaction be marked as rolled back only when exception is unhandled?

[rcd]

Bernard Labno wrote on Jan 19, 2012 03:12:

For JSF apps shouldn't transaction be marked as rolled back only when exception is unhandled?

That makes more sense to me. It could probably be accomplished by moving the rollback from the interceptor to a JSF ExceptionHandler, which will only be invoked if an unhandled exception occurs. I'll look into that as well, but again, it'll probably be a few days before I have time to go bug hunting.

[lightguard]

I would agree that this should be a mark for rollback. Then the transaction should be rolled back by the request listener in Transaction (as it currently stands).

[rcd]

Jason Porter wrote on Jan 19, 2012 15:55:


I would agree that this should be a mark for rollback. Then the transaction should be rolled back by the request listener in Transaction (as it currently stands).

Now I'm confused. Looking at the source, it appears to me that the PhaseListener is most certainly handling transaction start/commit/rollback, which duplicates what the ServletListener is also doing. Earlier, you implied that was the reason I was seeing a duplicate transaction (because the ServletListener was starting one and the PhaseListener was starting another) and said I needed to disable the ServletListener.

Although, that is an option to fix this problem: remove the PhaseListener, add the ExceptionHandler to mark for rollback, and let the ServletListener do all the work. The downside, of course, is that you lose fine-grained control over the transactions relative to the JSF lifecycle. You would only be able to start the transaction before JSF gets called, and only commit/rollback after JSF finishes, instead of being able to start/commit/rollback at the beginning/end of any JSF phase.

[lightguard]

Richard DiCroce wrote on Jan 19, 2012 16:19:


Now I'm confused. Looking at the source, it appears to me that the PhaseListener is most certainly handling transaction start/commit/rollback, which duplicates what the ServletListener is also doing. Earlier, you implied that was the reason I was seeing a duplicate transaction (because the ServletListener was starting one and the PhaseListener was starting another) and said I needed to disable the ServletListener.

Although, that is an option to fix this problem: remove the PhaseListener, add the ExceptionHandler to mark for rollback, and let the ServletListener do all the work. The downside, of course, is that you lose fine-grained control over the transactions relative to the JSF lifecycle. You would only be able to start the transaction before JSF gets called, and only commit/rollback after JSF finishes, instead of being able to start/commit/rollback at the beginning/end of any JSF phase.

Exactly. That's why I left the PhaseListener in, but I should have done some more work with it and simplified it. Not having the PhaseListener I think would be a bad thing. We wouldn't be able to commit the transaction before RENDER_RESPONSE, nor would we be able to switch to using Flush_Mode.MANUAL (in the case of Hibernate) during RENDER_RESPONSE.

Some background about adding the ServletRequestListener: I added it to fix https://issues.jboss.org/browse/SEAM-99. We had thought about using a filter, but then you get into the problem of ordering, a ServletRequestListener seemed like a better approach. From our understanding the problem occurred if an exception happened during a JSF request and the transaction was not rolled back. Then when a new request came in, often times being serviced using the same thread on the server (because it's HTTP 1.1 and the connection was still alive) it would die when it tried to start a new transaction because the previous one was never committed nor rolled back. The ServletRequestListener also seemed like a good idea as it allowed for transparent transactions for requests that weren't JSF requests such as Wicket, JAX-RS, GWT, etc. We could debate for a while if transparent transactions are a good thing or not, but I'll leave that for later.

I had thought to simplify the PhaseListener, but found it had support (maybe it works, maybe it doesn't, I'm honestly not sure) for declaring transactions in the ViewConfig feature. I didn't want to mess with things as that feature is documented and people may be using it now. Certainly if that PhaseListener can be simplified I'm all for it.

When I allowed for the ServletRequestListener to be disabled I noticed it had problems with Glassfish especially as the thread, and subsequently the transaction, was different leaving  the Listener than the one coming into the Listener. This confused me greatly, I don't know if that's a Glassfish bug or not though. I also thought if you're doing your own declarative transactions this might mess with that, so I thought it best to give the option of turning off the listener.

I know that's a big wall of text, but I hope it helps a little :)