Composable Locks using LINQ. Can anyone see any problem with this ?

Question

I was playing around with LINQ and I came up with the following idea for composing locks taking advantage of C# Monadic syntax. It seems too simple, so I thought let me post it on StackExchange and see if anyone can quickly spot any major problems with this approach.

If you comment the lock inside the atomic implementation you'll see the accounts getting corrupted.

This idea is about being able to compose operations on the accounts without explicit locking.

using System;
using System.Collections.Generic;
using System.Linq;

namespace AtomicLinq
{
    public static class AccountCombinators
    {
        public static IAtomic<Unit> TransferAndReverse(this Account accA, 
                                                      Account accB, 
                                                      double amount)
        {
            return from a in accA.TransferTo(accB, amount)
                   from b in accB.TransferTo(accA, amount)
                   select new Unit();
        }

        public static IAtomic<Unit> TransferTo(this Account accA, 
                                              Account accB, 
                                              double amount)
        {
            return from a in accA.Withdraw(amount)
                   from b in accB.Deposit(amount)
                   select new Unit();
        }

        public static IAtomic<double> Withdraw(this Account acc, double amount)
        {
            return Atomic.Create(() => acc.Amount -= amount);
        }

        public static IAtomic<double> Deposit(this Account acc, double amount)
        {
            return Atomic.Create(() => acc.Amount += amount);
        }
    }

    static class Program
    {
        static void Main(string[] args)
        {
            var accA = new Account("John") { Amount = 100.0 };
            var accB = new Account("Mark") { Amount = 200.0 };

            var syncObject = new object();
            Enumerable.Range(1, 100000).AsParallel().Select(_ => accA.TransferAndReverse(accB, 100).Execute(syncObject)).Run();

            Console.WriteLine("{0} {1}", accA, accA.Amount);
            Console.WriteLine("{0} {1}", accB, accB.Amount);

            Console.ReadLine();
        }        
    }    

    public class Account
    {
        public double Amount { get; set; }
        private readonly string _name;

        public Account(string name)
        {
            _name = name;
        }

        public override string ToString()
        {
            return _name;
        }
    }

    #region Atomic Implementation

    public interface IAtomic<T>
    {
        T Execute(object sync);
    }

    public static class Atomic
    {
        public static IAtomic<T> Create<T>(Func<object, T> f)
        {
            return new AnonymousAtomic<T>(f);
        }

        public static IAtomic<T> Create<T>(Func<T> f)
        {
            return Create(_ => f());
        }

        public static IAtomic<T> Aggregate<T>(this IEnumerable<IAtomic<T>> xs)
        {
            return xs.Aggregate((x, y) => from a in x
                                          from b in y
                                          select b);
        }

        public static IAtomic<K> SelectMany<T, V, K>(this IAtomic<T> m, Func<T, IAtomic<V>> f, Func<T, V, K> p)
        {
            return Create(sync =>
                              {
                                  var t = m.Execute(sync);
                                  var x = f(t);
                                  return p(t, x.Execute(sync));
                              });
        }
    }

    public class AnonymousAtomic<T> : IAtomic<T>
    {
        private readonly Func<object, T> _func;

        public AnonymousAtomic(Func<object, T> func)
        {
            _func = func;
        }

        public T Execute(object sync)
        {
            lock (sync)  // Try to comment this lock you'll see that the accounts get corrupted
                return _func(sync);
        }
    }

    #endregion Atomic Implementation   
}

Answer 1

The disadvantage I see in your design is that you have decoupled the lock from the actual objects that should not be changed by another thread during the critical section.
In your case only the two accounts of a transaction should be locked but you created a global lock syncObject that practically turns your parallel execution into a serial one. In other word if you had an accC and accD in Main you could not do a transfer between accC and accD in parallel with a transfer between accA and accB.
You might be able to hack this by creating a series of lock objects and pass them somehow one-by-one in the proper order to Execute but if the call to the paralell execution is made from a much higher abstraction level then the manipulated objects are on you probably won't be able to pass the proper lock objects to Execute. Your only option would be to use a global lock for that AsParallel() "session" and that would defeat the purpose of using it.

Answer 2

I'm trying to compile your code:

• What kind of class is Unit? Do I need to reference an additional assembly?
• Run at the end of Enumerable.Range(1, 100000).AsParallel().Select(_ => accA.TransferAndReverse(accB, 100).Execute(syncObject)).Run(); seems to be unnecessary.