Beware of locking helper functions in swift

If, like me, you are a swift programmer with a background in basically any other programming language, you'll be familiar with the lock statement.

C# Example:

class MyClass {
    private bool m_initialised = false;
    private int m_value = 0;

    void Initialise() {
        lock(this) { // lock stops two threads initialising at the same time
            if(m_initialised) {
                return; // already initialised
            }

            m_value = LoadExpensiveValue();
            m_initialised = true;
        }
        Console.WriteLine("initialised");
    }
}

Java has synchronized, heck even Objective-C has @synchronized - but swift has no such thing.

If you google for it, you most probably wind up on stack overflow, probably at this article: What is the Swift equivalent to Objective-C's “@synchronized”?.

It boils down to the following

1. Use objc_sync_enter to lock
2. Use objc_sync_exit to unlock
3. Don't forget to use defer to make sure the unlock always happens

Now, if you simply do the above, it's fine, however you (like me) are most probably tempted by that answer on that stack overflow post, which shows that you can write your own synchronised/lock function. Once you apply all the bells and whistles, you get something like this

Swift:

func lock(lockObj: AnyObject, closure: () throws ->T ) rethrows -> T
{
  objc_sync_enter(lockObj)
  defer { objc_sync_exit(lockObj) }
  return try closure()
}

And you can use it like this!

Swift Example:

class MyClass {
    private var _initialised = false
    private var _value = 0

    func initialise() {
        lock(self) { // lock stops two threads initialising at the same time
            if(_initialised) {
                return // already initialised
            }

            _value = loadExpensiveValue();
            _initialised = true
        }
        print("initialised");
    }
}

How cool is that? Swift didn't have a lock keyword, but we were able to write one ourselves, and it's just as nice to use as the native ones from C#, Java or anywhere else! Hooray!

Unfortunately, it is also wrong and broken

The above code compiles with zero errors or warnings, and there are many cases in which you'll never realise there's a bug there. I was bitten by this exact thing just today, hence my motivation to write this post.

The bug is, in the swift implementation, the print("initialised"); runs when _initialised is true; If two threads call the function, they will *both* print it.

Why? In swift, our lock keyword isn't a real keyword. It is a function which is being passed a closure.
That is to say, the locked region is a closure, and not part of the outer function.

The return statement for the "already initialised" case returns from the closure, but does not return from the outer initialise() function itself. This is in contrast to languages with a real lock keyword, which does not introduce a new return scope, and where the return statement actually returns from the outer function.

My advice: Do not write or use this handy swift lock function. One day you too will write this bug, and when it occurs it's not at all obvious as to what's going on.

Do this instead.

class MyClass {
    private var _initialised = false
    private var _value = 0

    func initialise() {
        do {
            objc_sync_enter(self); defer{ objc_sync_exit(self) }

            if(_initialised) {
                return // already initialised
            }

            _value = loadExpensiveValue();
            _initialised = true
        }
        print("initialised");
    }
}

The code is very slightly longer and very slightly uglier, however there are no closures, and the return keyword works how you expect it to.

Perhaps one day swift will add a proper lock keyword and we can all move on from this kind of thing