rebus-org/Rebus.TransactionScopes

await call in handler makes Transaction.Current lose its context

thomasdc opened this issue · 5 comments

When awaiting code in a handler that's decorated with an ambient TransactionScope, Transaction.Current looses its context.
Probably it's because of this line that reassigns Transaction.Current. I guess the TransactionScopeAsyncFlowOption isn't maintained when using the setter on Transaction.Current.

As mentioned here Transaction.Current is ThreadStatic and all code should preferably manipulate an ambient context via a using statement on TransactionScope.

I have a failing test that reproduces the issue.

Can you confirm the bug?
Is there a reason you removed the using statement in this commit?

I have a small Linqpad script reproducing the problem:

async Task Main()
{
	Thread.CurrentThread.ManagedThreadId.Dump("before await");
	var scope = new TransactionScope(TransactionScopeOption.Required, TransactionScopeAsyncFlowOption.Enabled);
	var transaction = Transaction.Current;
	
	await Task.Delay(1000).ConfigureAwait(false);
	
	Thread.CurrentThread.ManagedThreadId.Dump("after first await");
	Transaction.Current = transaction;
	Transaction.Current.Dump(); // not null
	
	await Task.Delay(1000).ConfigureAwait(false);
	
	Thread.CurrentThread.ManagedThreadId.Dump("after second await");
	Transaction.Current.Dump(); // is null :/
}

Damn. As the commit comment says, the change made the transport's receive operation be executed within a transaction scope too... so e.g. if you were using the SQL transport, then you could enlist all of your work in the same transaction.

Do you have any ideas on how this could be fixed?

Btw I see you posted the issue with Transaction.Current on StackOverflow too... even Stephen Cleary seems surprised by this behavior 😁

I managed to fix the unit test by adding a new TransactionScope wrapping the Transaction you refer to via ITransactionContext:

class TransactionScopeIncomingStep : IIncomingStep
    {
        ...
        public async Task Process(IncomingStepContext context, Func<Task> next)
        {
            ...
            using (var scope = new TransactionScope(transaction, TransactionScopeAsyncFlowOption.Enabled))
            {
                await next();
            }
        }
    }

However, the scope never completes and the message is never marked as processed.

See the reference commit where I tried to fix the issue.
The unit test now passes, but throws internally:

I'm really clueless on this one.