NSLock.Lock plus Await plus NSLock.Unlock leads to main thread freeze

The following code emulates case, when author of external library introduces locks, that could encompass await call. Is there any way, to prevent such cases (noasync annotation isn't solution, because: 1) it don't work if function wrapped in another function without noasync annotation; 2) author of third party library could forget to add such annotation).

let lock = NSLock()

func thirdPartyLibLock() {
    print("- do sum work and lock")
    lock.lock()
    /*
     I also tried to replace it with:
         await withCheckedContinuation({ c in
             lock.lock()
             c.resume()
         })
     */
}

func thirdPartyLibUnlock() {
    print("- do sum work and unlock")
    lock.unlock()
    /*
     I also tried to replace it with:
         await withCheckedContinuation({ c in
             lock.unlock()
             c.resume()
         })
     */
}

func example() {
    /*
     Console:
     - start 4
     - do sum work and lock
     - start 1
     - do sum work and lock

     And that's all. We have suspended main thread.
     Numbers 4 and 1 could differ between app launches, it's ok.
     */
    for i in 0...1000 {
        Task {
            print("- start \(i)")

            thirdPartyLibLock()
            try await Task.sleep(for: .seconds(1))
            thirdPartyLibUnlock()

            print("- end \(i)")
        }
    }

    // Won't be executed.
    DispatchQueue.main.asyncAfter(deadline: .now() + 3, execute: {
        print("- ping")
    })
}

Try:

print(Thread.current)
try await Task.sleep(for: .seconds(1))
print(Thread.current)

I guess you are on a different thread after await.

Not sure what's the end goal. For example, you may allocate a separate thread, call the third party code on that thread and to sleep you could use "Thread.sleep(forTimeInterval: 1)"

While tera has it right that after a call to Task.sleep the executing thread might have changed, it is not the root issue here.

Once you have acquired the "third party lock" and the first task suspends to sleep, the other tasks try to acquire the lock. Since the concurrency thread pool is limited every single thread can (and in this case will) be blocked on trying to acquire the lock. Now there are no more threads left to resume the first sleeping task and thus you have a deadlock.

You can write a wrapper around this "third party lock" with an MainActor isolated class so that the locking/unlocking is called from the same thread i.e. main thread

@MainActor
final class ThirdPartyLockWrapper {
    var locked = false
    var continuations = [CheckedContinuation<Void, Never>]()

    func lock() async {
        if !locked && continuations.isEmpty {
            locked = true
            thirdPartyLibLock()
            return
        } else {
            await withCheckedContinuation {
                continuations.append($0)
            }
        }
        continuations.removeFirst()
        locked = true
        thirdPartyLibLock()
    }

    func unlock() {
        locked = false
        thirdPartyLibUnlock()
        continuations.first?.resume()
    }
}

and then use that instead

...
let thirdPartyLockWrapper = ThirdPartyLockWrapper()
...
await thirdPartyLockWrapper.lock()
try await Task.sleep(for: .seconds(1))
await thirdPartyLockWrapper.unlock()

I have assumed that no other piece of code will call the thirdPartyLibLock() thirdPartyLibUnlock() methods. If that's not the case then the above will not work.

1 Like

yep, thank you for your solution.

So you suppose to wrap each method/function call of any library in such wrapper?

How it would work in real word, where anybody could call lock in any part of code that not protected by such wrapper?

lock+unlock API pairs are unsafe by design for a bunch of reasons, of which this is one. A better design is to have a single function which acquires the lock, calls a callback, and then releases the lock after the callback returns. (Ideally that function would also provide that callback access to the resource protected by the lock, which would otherwise be inaccessible.) The requirement to not suspend between the lock and unlock can very neatly be expressed by just making the whole thing synchronous.

8 Likes