2

When using Castle.Facilities.AutoTx facility with [Transaction(TransactionScopeOption.RequiresNew)] attribute, the expected new System.Transactions.CommittableTransaction is not created.

You can easily test it with the following unit test

using System.Transactions;
using Castle.Facilities.AutoTx.Testing;
using Castle.MicroKernel.Registration;
using Castle.Transactions;
using Castle.Windsor;
using NUnit.Framework;

namespace Castle.Facilities.AutoTx.Tests
{
    public class TransService
    {
        private readonly NewTransService _s2;

        public TransService(NewTransService s2)
        {
            _s2 = s2;
        }

        [Transaction]
        public virtual string DoInTrans()
        {
            var currentTransaction = System.Transactions.Transaction.Current;
            Assert.That(currentTransaction != null, "The current transaction mustn't be null.");
            string transId = currentTransaction.TransactionInformation.LocalIdentifier;

            _s2.DoInNewTrans(transId);

            return transId;
        }
    }

    public class NewTransService
    {
        [Transaction(TransactionScopeOption.RequiresNew)]
        public virtual string DoInNewTrans(string parentTransId)
        {
            var currentTransaction = System.Transactions.Transaction.Current;
            Assert.That(currentTransaction != null, "The current transaction mustn't be null.");
            string transId = currentTransaction.TransactionInformation.LocalIdentifier;

            Assert.AreNotEqual(parentTransId, transId, "Ambient transaction must differ from parent");

            return transId;
        }
    }

    public class SingleThread_NewAmbient
    {
        private WindsorContainer _Container;

        [SetUp]
        public void SetUp()
        {
            _Container = new WindsorContainer();
            _Container.AddFacility<AutoTxFacility>();
            _Container.Register(Component.For<TransService>());
            _Container.Register(Component.For<NewTransService>());
        }

        [TearDown]
        public void TearDown()
        {
            _Container.Dispose();
        }

        [Test]
        public void Automatically_Starts_New_CommitableTransaction()
        {
            using (var scope = new ResolveScope<TransService>(_Container))
                scope.Service.DoInTrans();
        }

    }
}

Am I misunderstanding the purpose of [Transaction(TransactionScopeOption.RequiresNew)] or is it a bug?

I have been digging into the Castle.Transactions source code and I was able to fix the behavior by changing following piece of code in Castle.Transactions.TransactionManager.ITransactionManager.CreateTransaction(ITransactionOptions transactionOptions):

if (activity.Count == 0)
    tx = new Transaction(new CommittableTransaction(new TransactionOptions
    ...

to

if (activity.Count == 0 || transactionOptions.Mode == TransactionScopeOption.RequiresNew)
    tx = new Transaction(new CommittableTransaction(new TransactionOptions
    ...

Can someone from Castle experts / owners check this?

Patrick Quirk
  • 23,334
  • 2
  • 57
  • 88
Pecan
  • 350
  • 3
  • 14
  • You may get a quicker answer from the devs if you submit a pull request with the change you mention. Although, that code base hasn't changed in 2-3 years so maybe not... – Patrick Quirk Aug 05 '14 at 17:43
  • @PatrickQuirk that's mostly because it's very stable software, but also because I've been exploring other methods of transaction composition the last three years. I merged a PR to the NHibernate integration for this project just a couple of days ago, so it seems people are still using it. We're e.g. using this code-base for a blasting system collecting blast probe-data/metrics, so it works for thousands of tranasctions per second. – Henrik Aug 06 '14 at 14:25
  • @Henrik Oh I certainly wasn't implying anything about the quality of the project or the lack of recent commits. There's a lot of inactive open source projects and it can be difficult for a passerby to determine if they're stable or the authors have abandoned it. Thanks for clarifying! – Patrick Quirk Aug 06 '14 at 14:36

1 Answers1

2

Author here,

I think your code is great and would merge a PR with it if it doesn't break other tests. =)

The reason that RequiresNew isn't well supported is because it's in 99% of the cases an anti-pattern. You're after encapsulating your unit of work in a transaction; and your unit of work should correspond 1-1 with the business operation that should be consistent.

Now, if you have a transaction going on the current thread, like you'd have in your case if you need to use 'RequiresNew', then you're either after reading dirty data or spawning an unrelated transaction (from a business-operation perspective). Hence, you should be doing it in another thread.

Since transactions are 'ambient' and not explicit in the control flow in a programming language like C#, you're left with your 'call context slots' to save the transaction reference in; but from your code's point of view these don't exist; what you have are partial functions that only work in a transactional context. If you spawn a second transaction; how are you going to coordinate that with the current transaction? It's hard; and likely to lead to problems.

An aside

In other transactional systems, like geteventstore.com you get an explicit transaction identifier -- you have one in System.Transactions, too, but it's not explicit in the API/ABI/ADO.Net to the database, hence you can't use it the same way. With an explicit transaction identifier you can solve failures when it becomes 'in doubt', i.e. the 'generals problem'; you can't with System.Transactions. Instead you must mount the transaction MMC on the DTC in question and manually roll it back or forward.

Henrik
  • 9,714
  • 5
  • 53
  • 87