NSLock broken when using optimize speed in XCode Swift?

Dear Sirs,

I've written a SwiftUI application in XCode where I'm using multiple threads which are synchronized by using NSLock and NSRecursiveLock. This works in Debug mode and in Release mode as long as I don't use the Swift Compiler - Code Generation -> Optimization Level -O for "Optimize for Speed". This is unfortunately the default setting but it results in multiple threads accessing the same piece of code which is encapsulated inside a NSLock. Is this an intended behaviour?

Thanks and best regards, Johannes

Replies

Is this an intended behaviour?

No. Show us your code.

To expand on endecotp’s accurate, albeit succinct, response…

The vast majority of times, weird concurrency bugs that only show up when you turn on optimisations are caused by a problem with the code rather than a problem with the compiler or a the system’s concurrency primitives (like NSLock). Writing correct concurrent code is hard [1], and it’s very easy to write code that relies on undefined behaviour.

If you post some details about the problem you’re seeing, we should be able to advise you further.

Share and Enjoy

Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

[1] A situation that Apple is aiming to improve with Swift concurrency.

Hi, I did some further investigation and I think the problem is due to the fact that I'm doing the locking within a class that releases the lock when it goes out of scope. So my class to do this looks like this:

class CRecursiveMutex
{
    let m_RecursiveLock: NSRecursiveLock
    
    init( _ RecursiveLock: NSRecursiveLock )
    {
        m_RecursiveLock = RecursiveLock
        m_RecursiveLock.lock()
    }
    
    deinit
    {
        m_RecursiveLock.unlock()
    }
    
//    func DoNothing() {
//    }
}

now I have another class using this:

    class CMyClass {

        var m_CSMyLock: NSRecursiveLock = NSRecursiveLock()

        public func DoSomething()
        {
            let Lock: CRecursiveMutex = CRecursiveMutex( m_CSMyLock )
            
//            ... do something

//            Lock.DoNothing()
        }
    }

So when you call DoSomething() from different threads the locking will not work in optimized mode, while it works correct if you disable the optimization. It will also work if you uncomment the function DoNothing() and the calls to it which I just added for testing purposes. So for me it looks like in the end the optimization seems to remove my local Lock variable probably as it thinks it is not used. I like to use this kind of locking by a class that goes out of scope as you can be sure the lock is released wherever you leave the function and I'm also using it in C++ classes where they also survive the compiler optimizations.

So my class to do this looks like this

Yeah, that’s not valid Swift code.

You’re trying to replicate the RAII pattern and Swift doesn’t support that. The issue is that there’s nothing holding your Lock local variable in scope and thus the optimised build creates it and then immediately releases it. Swift does not, for example, guarantee that it’ll stay around until the end of the enclosing scope.

If you want something to stay around you have to either reference it or use withExtendedLifetime(_:). However, I don’t recommend either of those because the standard pattern for this stuff in Swift is to provide a withXxx method that runs a body while the resource is held. And NSRecursiveLock supports this pattern directly. For example:

let lock = NSRecursiveLock()
lock.withLock {
    print("foo")
}

Some final notes:

  • Life will be easier if you adopt Swift naming conventions. Lock should be lock, m_CSMyLock should be myLock, DoSomething() should be doSomething(), and so on.

  • I consider recursive locks to be an anti-pattern. Reply back here if you’d like me to explain why (-:

Share and Enjoy

Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

Hi,

thanks for this explanation. It would be fine if XCode would mark this as an error if it is not supported. I already noticed that there's a difference when use a named variable versus an _ for this Lock variable, as using the underscore shows the same behaviour even in debug mode as the optimized version and just lets it go out of scope immediately. I'll switch to the withXxx version.

About your notes:

  • I know that the default naming is different, on the other hand if you're working in different languages (C,C++,Java,Swift,Typescript, etc.) and you want to follow each naming convention it may be also confusing. So sometimes when I know that my code will not become public, I'm reusing the naming convention I'm used to and I like best. For me in the end it's just names.

  • I think recursive locks are quite useful sometimes. Of course you shouldn't use them just because you don't know if you've already used this lock somewhere else. But I'd like to hear your explanation :-)

It would be fine if Xcode would mark this as an error if it is not supported.

Mark what specifically?

But I'd like to hear your explanation :-)

In an ideal world the concurrency context of your code should be subject to static analysis. So, you could look at your code and know what locks it holds just by the context in which it’s compiled. Indeed, that’s exactly what you get when you use Swift concurrency [1].

In lesser environments it’s common to establish a naming convention for this sort of this. For example, function foo() might take a lock, call function locked_foo(), and then release the lock. That way the name offers a hint as to whether you expect to be locked or not.

Recursively locks completely undermine that paradigm. The same code will work if it’s called from one context and fail if it’s call from another.

Moreover, recursive locks encourage folks to run large chunks of code under the lock, which is often problematic.

Share and Enjoy

Quinn “The Eskimo!” @ Developer Technical Support @ Apple
let myEmail = "eskimo" + "1" + "@" + "apple.com"

[1] And enable strict concurrency checking. And we’ve implemented all the features needed to do that reasonably, which is still a bit of a work in progress (-:

  • Recursive locks came up in another context, and someone forward me a link that I can't help but share: http://www.zaval.org/resources/library/butenhof1.html

Add a Comment