Tuesday, June 2, 2009

How to implement “lock” with timeout?

Anybody who did any multithreaded application probably used the “lock” keyword. This is actually a good thing.

“lock” is the most optimized way to lock a resource. First let’s take a look at what lock really do. Here is the simple Account class we will use for this post.

using System.Threading;

namespace Banking
{
    public class Account
    {
        private readonly string _accountNumber;
        private double _balance;

        private Account(string accountNumber, double amount)
        {
            _accountNumber = accountNumber;
            Balance = amount;
        }

        public string AccountNumber
        {
            get { return _accountNumber; }
        }

        public double Balance
        {
            get { return _balance; }
            set { _balance = value; }
        }

        public static Account OpenNew(string accountNumber, double amount)
        {
            return new Account(accountNumber, amount);
        }

        public void Deposit(double amount)
        {
            Interlocked.Exchange(ref _balance, _balance + amount);
        }

        public void Withdraw(double amount)
        {
            Interlocked.Exchange(ref _balance, _balance - amount);
        }

        public static void Transfer(double amount, Account fromAccount, Account toAccount)
        {
            // Bad code here. Potential deadlock.
            lock (fromAccount)
            lock (toAccount)
            {
                fromAccount.Withdraw(amount);
                toAccount.Deposit(amount);
            }
        }

        public override string ToString()
        {
            return string.Format("{0} (Balance = {1})", _accountNumber, _balance);
        }
    }
}

Here are some important things to point out about this class:

  • The constructor is private to limit the creators of this class.
  • The public OpenNew method is the only way to create an instance. This ensure that every Account starts with a name and a balance.
  • Deposit and Withdraw methods are thread safe. They both uses Interlocked class which is has low level methods to modify values.
  • Transfer is not thread safe even though it uses locks. There is a potential deadlock if two thread transfer funds using the same accounts at the same time.

Most of the time the transfer method will work without any problem but the is a slight chance of deadlock. Of course this is a fairly simple method and in fact we can use another private object field to lock on like in this sample.

private static object _syncLock = new object();

public static void Transfer(double amount, Account fromAccount, Account toAccount)
{
    // Bad code here. Potential deadlock.
    lock (_syncLock)
    {
        fromAccount.Withdraw(amount);
        toAccount.Deposit(amount);
    }
}

That mean you will need to use that _syncLock object all the time to be consistent, even if you need to lock only one of the two accounts. And because Transfer is a static method we need to make the _syncLock object static too. That mean that any other call to Transfer will have to wait until this call finish. That is a huge performance issue.

What we really need is to be able to lock actual Account objects and recover from any deadlock. The best way to do this is to use timeouts on the locking process. The caller can catch timeouts and handle it properly instead of waiting forever. Here is a better Transfer method.

public static void Transfer(double amount, Account fromAccount, Account toAccount)
{
    bool fromLock = Monitor.TryEnter(fromAccount, 1000);
    bool toLock = Monitor.TryEnter(toAccount, 1000);
    try
    {
        if (fromLock && toLock)
        {
            fromAccount.Withdraw(amount);
            toAccount.Deposit(amount);
        }
    }
    finally
    {
        if (fromLock)
            Monitor.Exit(fromLock);

        if (toLock)
            Monitor.Exit(toLock);
    }
}

This is a lot more code to write. Actually this is not far from what the lock keyword would do. Because if you look at your code with Reflector you will see that lock does translate to Monitor.Enter and Monitor.Exit (your have to look in IL not in C#). So a simple lock statement like this.

lock(obj)
{
    // do somtehing
    Console.WriteLine("Locked");
}

Would be translated to.

Monitor.Enter(obj);
try
{
    // do somtehing
    Console.WriteLine("Locked");
}
finally
{
    Monitor.Exit(obj);
}

You won’t see it in C# (with Reflector) but if you look at IL code you will see exactly the same sequence twice.

So to solve our problem, would it be nice to have something like this?

public static void Transfer(double amount, Account fromAccount, Account toAccount)
{
    Safe.Lock(new [] {fromAccount, toAccount}, 1000, () =>
    {
        fromAccount.Withdraw(amount);
        toAccount.Deposit(amount);
    });
}

This look pretty much like the well known lock keyword, isn’t it? Now how can we do this? That’s better. The Safe static call has a Lock static method that take care of everything. The fist argument can be a single object or an array of objects. This allow locking multiple object at once. The Lock method use the Monitor.TryEnter to acquire lock on objects. If this can be done before the timeout occur the Action argument is executed.

Of course this code can and should be surrounded with a try-catch block. here is how to do so.

public static void Transfer(double amount, Account fromAccount, Account toAccount)
{
    var retries = 10;

    while (retries-- > 0)
    {
        try
        {
            Safe.Lock(new[] { fromAccount, toAccount }, 1000, () =>
            {
                fromAccount.Withdraw(amount);
                toAccount.Deposit(amount);
            });
            break;
        }
        catch (TimeoutException e)
        {
            if (retries == 0)
                throw;
            Thread.Sleep(100);
        }
    }
}

In this code snippet it will retry at most 10 time to lock toAccount and fromAccount before giving up. Notice that only the TimeoutException is handled, so any other exception will be thrown immediately and stop the process. The break at the end of the try block let us out as soon as it works.

This is a good timeout pattern and easy to implement in your own code. Give it a try and let me know if it works.

Here is the full code for the Safe class.

using System;
using System.Linq;
using System.Threading;

namespace MultithreadHelper
{
    public class Safe : IDisposable
    {
        private readonly object[] _padlocks;
        private readonly bool[] _securedFlags;

        private Safe(object padlock, int milliSecondTimeout)
        {
            _padlocks = new[] {padlock};
            _securedFlags = new[] {Monitor.TryEnter(padlock, milliSecondTimeout)};
        }

        private Safe(object[] padlocks, int milliSecondTimeout)
        {
            _padlocks = padlocks;
            _securedFlags = new bool[_padlocks.Length];
            for (int i = 0; i < _padlocks.Length; i++)
                _securedFlags[i] = Monitor.TryEnter(padlocks[i], milliSecondTimeout);
        }

        public bool Secured
        {
            get { return _securedFlags.All(s => s); }
        }

        public static void Lock(object[] padlocks, int millisecondTimeout, Action codeToRun)
        {
            using (var bolt = new Safe(padlocks, millisecondTimeout))
                if (bolt.Secured)
                    codeToRun();
                else
                    throw new TimeoutException(string.Format("Safe.Lock wasn't able to acquire a lock in {0}ms",
                                                             millisecondTimeout));
        }

        public static void Lock(object padlock, int millisecondTimeout, Action codeToRun)
        {
            using (var bolt = new Safe(padlock, millisecondTimeout))
                if (bolt.Secured)
                    codeToRun();
                else
                    throw new TimeoutException(string.Format("Safe.Lock wasn't able to acquire a lock in {0}ms",
                                                             millisecondTimeout));
        }

        #region Implementation of IDisposable

        public void Dispose()
        {
            for (int i = 0; i < _securedFlags.Length; i++)
                if (_securedFlags[i])
                {
                    Monitor.Exit(_padlocks[i]);
                    _securedFlags[i] = false;
                }
        }

        #endregion
    }
}

7 comments:

fe said...

very nice!

I wonder if WaitHandle.WaitAll might reflect the intent (and likelyhood) to aquire all locks or none.

There is a case (aquiring the locks in a for loop) that two threads running at the same time will keep failing - I susepect that you might want to consider a variable sleep time, otherwise the threads will just wake up at the same time...

Eric De C# said...

I'm working on a fluent interface API version of this Safe.Lock. Here are some sample uses:
Safe.Lock(myObject).Execute(o => {
o.DoIt()
});
Safe.Lock(myObject).Timeout(1000).Execute(o => {
o.DoIt()
});
Safe.Lock(myObject).Timeout(1000).OnTimeout(o => {
o.Recover()
}).Execute(o => {
o.DoIt()
});

Avi said...

thank you

Dave said...

I'm very late to the party here, but this is outstanding work. Just what I needed. Thanks!

Dan said...

Very nice code, thanks!
Just one question when using an array of objects to be locked. The timeout seems to be used for the lock of each object from the array (the Monitor.TryEnter loop). That means that if we have for instance several objects and each of them is acquired after lets say 90% of the timeout value, in the sum the "Safe.Lock" method will wait much more than the configured timeout, without throwing the TimeoutException.

Eric De C# said...

Good point. You can try this improvement:

private Safe(object[] padlocks, int milliSecondTimeout)
{
_padlocks = padlocks;
_securedFlags = new bool[_padlocks.Length];
using (var timeoutRemaining = new Countdown(milliSecondTimeout))
{
for (int i = 0; i < _padlocks.Length; i++)
{
_securedFlags[i] = Monitor.TryEnter(padlocks[i], timeoutRemaining.RemainingMilliseconds);
}
}
}

I haven't tested that solution but all you need to do is to create a Countdown class that will give you the remaining time and it should work.

Dave Black said...

I see a problem though with the implementation where it relies on a programmer using an instance of this class correctly - which we know doesn't always work - that's why code has bugs!

The problem is that the implementation requires a creator of an instance to call Dispose() or implement a "using()" statement. Otherwise, the lock will never be released and you will hang the thread. The CLR never calls Dispose() as part of it's GC algorithm - see my answers provided to these StackOverflow posts What happens if I don't call Dispose on the pen object? and Dispose, when is it called?