Action Filter Thread Safety Routing Issues - invoking delegate.call(ctx) from a Promise

[Joseph Lust]

Play peeps,

I've been using Play to build an API backend at work. Everything has been working fine for 2 months (still in QA), but when I fire up ApacheBench, I start getting some unholy behavior.

Problem: Requests destined for one action are routed to the wrong action, only when under high concurrent load

Test Case:

• Run ApacheBench against an API EndPointA or EndPointB (not both at the same time), N=1000, concurrent connections=25
• Result -> 100% passing (proper length, proper return code)
• Run ApacheBench against 2 API endpoints, EndPointA: N=1000, concurrent connections=25, EndPointB: N=1000, concurrent connections=25
• Result -> ~10% of requests fail, inspection shows failures are error codes from wrong endpoint (i.e. request with EndPointA params sent to EndPointB), because request was mis routed

Obvious Culprit

I found the code causing this in a composited action filter a developer copied from online. Clearly the same authentication action (debugging indicates there is only 1 instance) is shared among all requests and the delegate object changed before async return.

Making a copy of the delegate reference before doing an async return almost fixed the problem.

Now 0.5% of requests are misrouted (using test N=20000). Any ideas what I'm missing? Thanks.

Code before fix:

@Named
public class HttpBasicAuthAction extends play.mvc.Action<HttpBasicAuth> {

    @Inject
    private UserDao userDao;

    @Override
    public F.Promise<Result> call(final Http.Context context) throws Throwable {

        final Optional<Credentials> inputCredentials = parseRequestCredentials(context);

        // check username and password
        return userDao.getUserPassword(inputCredentials.getUsername()).flatMap( userPwd -> {

            if (userPwd.isPresent()) {

                // set user or fail out
                if (cachedPwd.equals(inputCredentials.getPassword())) {
                    // set user
                    Context.setUserId(context, untrustedCreds.getUsername());
                    return delegate.call(context);
                }
            }
            return F.Promise.<Result>pure(unauthorized());
        });
    }
}

Code after fix:

@Named
public class HttpBasicAuthAction extends play.mvc.Action<HttpBasicAuth> {

    @Inject
    private UserDao userDao;

    @Override
    public F.Promise<Result> call(final Http.Context context) throws Throwable {

        final Action<?> myDelegate = delegate; // save final reference because can change during async calls

        final Optional<Credentials> inputCredentials = parseRequestCredentials(context);

        // check username and password 
        return userDao.getUserPassword(inputCredentials.getUsername()).flatMap( userPwd -> {

            if (userPwd.isPresent()) {

                // set user or fail out
                if (cachedPwd.equals(inputCredentials.getPassword())) {
                    // set user
                    Context.setUserId(context, untrustedCreds.getUsername());
                    return myDelegate.call(context);
                }
            }
            return F.Promise.<Result>pure(unauthorized());
        });
    }
} 

[Joseph Lust]

No one bit. After pulling the thing apart I discovered the true cause of the problem was Spring DI. While the Action was never declared a Singleton, I'd assumed that the @With annotation was being handled by Play and requesting a new instance of HttpBasicAuthAction as needed from the DI framework. This is however not the case and defaults to a Singleton.

Solution:

@Scope("prototype")

I've added a PR to the Play Doc's since the present Action docs would leave one none the wiser. There are also Issues against this lack of documentation in Play, which have been ignored. I hope this gets marked as a clearer problem, since as written many of the Java examples will fail spectacularly under any kind of heterogeneous, concurrent request loading.

-Joe