add lock to improve concurrency usage

[ahernandezlopez]

TL;DR

• add lock to improve concurrency usage
• added tests to check concurrency usage is correct

The change is motivated because we found a crash in the letgo app (using keychain-swift). We found out that the reason is accessing the keychain concurrently from two threads. The crash is happening in getData method when calling SecItemCopyMatching.

We implemented some tests in ConcurrencyTests.swift. If removing the NSLock usage and running them you'll see the above mentioned crash. Contribution by @elikohen

[evgenyneu]

Hi, thanks for contributing. A couple of questions:

• What motivated this change? Did it crash without the lock?
• How safe is it to use NSLock? Is it possible that it will create dead locks?

[ahernandezlopez]

Hi! Thanks! The contribution is by @elikohen so credit is for him ;) We have this improvement in our fork so I thought it would be nicer to have it in the main repo.

About your questions:

1. The change is motivated because we found a crash in the letgo app (using keychain-swift). We found out that the reason is accessing the keychain concurrently from two threads. The crash is happening in getData method when calling SecItemCopyMatching. Yes, it crashes without the lock. You will be able to see the crash when running ConcurrencyTests.swift.
2. It is safe. We lock at the beginning of the method and always unlock using a defer clause. Currently, it is not possible to create a dead lock as the only place to acquire the lock is in that method.

[ahernandezlopez]

Updated PR description :)

[ahernandezlopez]

Thanks @evgenyneu!

[evgenyneu]

Thanks for awesome fix @elikohen and @ahernandezlopez! I've released it in version 10 to cocoapods.

[ahernandezlopez]

Cool! Thanks!!!