@Transactional and TransactionalAction conception problem - does not work with Secure Social plugin

[rlamarche]

Hi,

I've found a critical bug in the play java ebean plugin. In my opinion, there is a confusion in the usage of promise and action wrappers (@With ...).

Look at this portion of code from the class play.db.ebean.TransactionalAction: (https://github.com/playframework/play-ebean/blob/268c771426398353af39c0c45fb458122cc57a6b/play-ebean/src/main/java/play/db/ebean/TransactionalAction.java)

/**
 * Wraps an action in an Ebean transaction.
 */
public class TransactionalAction extends Action<Transactional> {

    public F.Promise<Result> call(final Context ctx) throws Throwable {
        return Ebean.execute(new TxCallable<F.Promise<Result>>() {
            public F.Promise<Result> call() {
                try {
                    return delegate.call(ctx);
                } catch (RuntimeException e) {
                    throw e;
                } catch (Throwable t) {
                    throw new RuntimeException(t);
                }
            }
        });
    }

}

The EBean transaction starts before entering the public F.Promise<Result> call() method, and is commited or rollbacked depending if there is an exception thrown at the end of call().

In this callable, the result of delegate.call(ctx) is returned. This result is of kind F.Promise<Result>. A promise is a "promise of response", but nothing tell us that the underlying action has already been executed after the delegate call. Moreover, nothing tell us that it will be executed in the same thread.

In the common case where no play plugins are used, this portion of code works: but I insist on the point that this case is a special case, because I don't know why but when the F.Promise is returned, the underlying action has already been executed in the current thread. I think that this is a misconception but that's my opinion. Moreover nothing forbid in the current API to execute the delegate action in another thread and later (I mean after the transaction commit).

For example, if you use the popular and almost vital plugin Secure Social, this code does not work anymore. There is no exception thrown, but the subsequent action is executed outside the transaction, and this is a very bad idea.

Indeed, in this case, the delegate returns a F.Promise where the action has not yet been executed. And again, I insist on the point that this is not a Secure Social bug in my opinion because the promise API is made to allow that!

For information, I'm using Secure Social 3.0-M3 and Play Framework 2.3.8.

Currently, I'm trying to understand how to really "wrap" the action execution using the F.Promise API, but any help would be really appreciated.

[rlamarche]

Note that EBean is using a ThreadLocal internally to store the current transaction, like a lot of ORMs.

[lfleuriot]

:+1:

[jroper]

There are a number of issues here. Firstly, I think it is possible to provide a custom thread local scope for ebean, such that the transaction is bound to Play's http context, and then have the transaction committed/rolled back in the callbacks attached to the returned future. However, doing this is prone to deadlocks, due to the fact that the action could block on the connection pool, while the request with a connection could be stuck in the thread pool queue waiting for the thread blocking on the connection pool to finish executing.

The best approach for now is to explicitly use Ebean.execute yourself. I think a proper solution to this would be to create an AsyncTransactional annotation, with clear docs on it about the potential deadlocks that could occur.