[TransactionAttribute] not working with async programming

[djechelon]

Hi,

I have incidentally found that using AOP attributes with async programming might not as straightforward as it looks. Consider an example code (EF6 based, which works with Txes)

[Transaction(ReadOnly = true)]
public async Task<boolean> UserExists(string username){
    using (SomethingDbContext ctx = new SomethingDbContext(...)) {
        return ctx.Users.AnyAsync(x=> x.username == username);
    }
}

Invoking this method like await UserRepository.UserExists(someUserName); throws a DataException saying the transaction is closed, most likely because the effects of AOP Transaction happen before the actual Any method is executed.

Personally, I gave up aynchronous programming for classing synchronous (maybe old school multi-threaded in some cases) programming but I'd like to share my finding with the community, so maybe if this cannot be avoided at least it could be documented (Spring docs look pretty old, there is no trace of asynchronous programming).

[sbohlen]

Thanks for the feedback.

TBH, I don't think that this is limited in any meaningful way to TX advice per se. Most advice (TX or otherwise) that is of type AROUND or AFTER (i.e., any advice that would technically need to wait for deferred execution of the method in order to perform its AFTER-invocation-action) would be expected to work with asyc methods without modification. The advice itself would almost certainly have to be modified to be 'async-aware' (i.e., to await the advised method call prior to continuing its AFTER-invocation behavior) in order for this to be expected to work properly.

If you want to explore what might be required for AFTER or AROUND advice to properly support async-invocations, we'd be happy to consider a pull-request!

[djechelon]

Hi,

I don't think I will contribute to this one (too many things to do, too little experience in AOP) but here is my idea for anyone who wishes to cooperate.

Since [Transaction] is basically mapped to an Around advice, so that it opens the Tx Before and commits After, just to keep it simple, it could be an idea to detect wheter the invoked method is synchronous or asynchronous (at least you can check if return type is Task). If so, instead of executing actions, the AOP engine should wrap the entire Task returned by the adviced method with an "outer" Task that basically does something like...

public Task AroundAdvisorTask(Task innerTask) {
    DoSomethingBefore();
    await innerTask;
    DoSomethingAfter();
}

This method, returned as value to a caller doing await TransactionalMethod() might produce the expected result.

[sbohlen]

Thanks for the suggestions. That straightforward pattern might perhaps be feasible for general AROUND advice (and sim. for AFTER advice).

Unfortunately, my quick guess is that it would likely be insufficient (without further changes elsewhere) in the specific case of Tx advice. Classes like the various IPlatformTransactionManager implementations and TransactionSynchronizationManager and it subclasses make a number of assumptions about thread-affinity (i.e., for persisting suspended Tx, etc.) that would no longer hold true in scenarios where the advised method is an async invocation.

These supporting classes (and probably others as well) would likely also either need to become 'async-aware' or have async-supporting variants of themselves introduced in order to support Tx advice around any async invocations.

I don't see any technical reason why this couldn't be achieved, but unfortunately its probably not as simple/quick an undertaking as one might hope :(

[djechelon]

Hi again,

for the sake of discussion I don't fully agree with your statement about IPlatformTransactionManager.

Async programming is not multithreading so that every statement is bound only to a single thread, then I see no reason for Tx not working because of thread affinity.

Instead, a reason for which Tx won't work in async scenario is the following:

Task[] batchJobs = new Task[] {DoJob1(), DoJob2(), DoJob3());
await Task.WhenAll(batchJobs);

This because if jobs are transacted in REQUIRED propagation, the Tx manager won't be able to open 3 distinct transactions.

But the good news is that I believe that this problem cannot be overcome even in pure EF coding, so that's not a limitation for Spring only. This is to say that the only way to batch multiple SQL operations on DB to save execution time is plain-old-school multithreading.

[sbohlen]

Sort of :)

See e.g., http://blog.stephencleary.com/2013/04/implicit-async-context-asynclocal.html, http://stackoverflow.com/questions/23555255/how-to-maintain-thread-context-across-async-await-model-in-c as just simple examples. TransactionSynchronizationManager makes use of CallContext.Get/SetData for its persistence mechanism and this won't provide the desired behavior under async invocations.

Async's thread-affinity is within a single async/await 'chain', not across them, which is what would (likely) blow up when anyone tries to nest/promote transactions (obviously a common and necessary use-case for the Spring Tx advice).

The point isn't as much that this is hard per se, but that its merely different and will have a tendency to spider its way across a larger number of supporting classes rather than being addressable merely by making the advice async ... the classes that manage/interact with the advised types need to be made 'async-aware' as well and this is (probably) a non-trivial undertaking.

[robertovaldesperez]

Hi, is there a TransactionAttribute solution for NHibernate 5.0?

[lahma]

I'm doing a rude experiment with closing this, like what stale bot does, but maybe more human as I will probably actually react to feedback. As this issue has been stale for so long, I'll close it. If it's still an issue you would like to pursue, we can definitely reopen.

With limited resources this is just something that we need to do. Thank you for your understanding.