How to Implement a Thread Safe Hashtable (Phonebook) Data Structure in Swift

How to implement a Thread Safe HashTable (PhoneBook) Data Structure in Swift?

The problem is your ReaderWriterLock. You are saving the writeClosure as a property, and then asynchronously dispatching a closure that calls that saved property. But if another exclusiveWrite came in during the intervening period of time, your writeClosure property would be replaced with the new closure.

In this case, it means that you can be adding the same Person multiple times. And because you're using a dictionary, those duplicates have the same key, and therefore don't result in you're seeing all 1000 entries.

You can actually simplify ReaderWriterLock, completely eliminating that property. I’d also make concurrentRead a generic, returning the value (just like sync does), and rethrowing any errors (if any).

public class ReaderWriterLock {
private let queue = DispatchQueue(label: "com.domain.app.rwLock", attributes: .concurrent)

public func concurrentlyRead<T>(_ block: (() throws -> T)) rethrows -> T {
return try queue.sync {
try block()
}
}

public func exclusivelyWrite(_ block: @escaping (() -> Void)) {
queue.async(flags: .barrier) {
block()
}
}
}

A couple of other, unrelated observations:

  1. By the way, this simplified ReaderWriterLock happens to solves another concern. That writeClosure property, which we've now removed, could have easily introduced a strong reference cycle.

    Yes, you were scrupulous about using [weak self], so there wasn't any strong reference cycle, but it was possible. I would advise that wherever you employ a closure property, that you set that closure property to nil when you're done with it, so any strong references that closure may have accidentally entailed will be resolved. That way a persistent strong reference cycle is never possible. (Plus, the closure itself and any local variables or other external references it has will be resolved.)

  2. You're sleeping for 10 seconds. That should be more than enough, but I'd advise against just adding random sleep calls (because you never can be 100% sure). Fortunately, you have a concurrent queue, so you can use that:

    concurrentTestQueue.async(flags: .barrier) { 
    print(phoneBook.count)
    }

    Because of that barrier, it will wait until everything else you put on that queue is done.

  3. Note, I did not just print nameToPersonMap.count. This array has been carefully synchronized within PhoneBook, so you can't just let random, external classes access it directly without synchronization.

    Whenever you have some property which you're synchronizing internally, it should be private and then create a thread-safe function/variable to retrieve whatever you need:

    public class PhoneBook {

    private var nameToPersonMap = [String: Person]()
    private var phoneNumberToPersonMap = [String: Person]()

    ...

    var count: Int {
    return readWriteLock.concurrentlyRead {
    nameToPersonMap.count
    }
    }
    }
  4. You say you're testing thread safety, but then created PhoneBook with .none option (achieving no thread-safety). In that scenario, I'd expect problems. You have to create your PhoneBook with the .threadSafe option.

  5. You have a number of strongSelf patterns. That's rather unswifty. It is generally not needed in Swift as you can use [weak self] and then just do optional chaining.

Pulling all of this together, here is my final playground:

PlaygroundPage.current.needsIndefiniteExecution = true

public class Person {
public let name: String
public let phoneNumber: String

public init(name: String, phoneNumber: String) {
self.name = name
self.phoneNumber = phoneNumber
}

public static func uniquePerson() -> Person {
let randomID = UUID().uuidString
return Person(name: randomID, phoneNumber: randomID)
}
}

extension Person: CustomStringConvertible {
public var description: String {
return "Person: \(name), \(phoneNumber)"
}
}

public enum ThreadSafety { // Changed the name from Qos, because this has nothing to do with quality of service, but is just a question of thread safety
case threadSafe, none
}

public class PhoneBook {

private var threadSafety: ThreadSafety
private var nameToPersonMap = [String: Person]() // if you're synchronizing these, you really shouldn't expose them to the public
private var phoneNumberToPersonMap = [String: Person]() // if you're synchronizing these, you really shouldn't expose them to the public
private var readWriteLock = ReaderWriterLock()

public init(_ threadSafety: ThreadSafety) {
self.threadSafety = threadSafety
}

public func personByName(_ name: String) -> Person? {
if threadSafety == .threadSafe {
return readWriteLock.concurrentlyRead { [weak self] in
self?.nameToPersonMap[name]
}
} else {
return nameToPersonMap[name]
}
}

public func personByPhoneNumber(_ phoneNumber: String) -> Person? {
if threadSafety == .threadSafe {
return readWriteLock.concurrentlyRead { [weak self] in
self?.phoneNumberToPersonMap[phoneNumber]
}
} else {
return phoneNumberToPersonMap[phoneNumber]
}
}

public func addPerson(_ person: Person) {
if threadSafety == .threadSafe {
readWriteLock.exclusivelyWrite { [weak self] in
self?.nameToPersonMap[person.name] = person
self?.phoneNumberToPersonMap[person.phoneNumber] = person
}
} else {
nameToPersonMap[person.name] = person
phoneNumberToPersonMap[person.phoneNumber] = person
}
}

var count: Int {
return readWriteLock.concurrentlyRead {
nameToPersonMap.count
}
}
}

// A ReaderWriterLock implemented using GCD concurrent queue and barriers.

public class ReaderWriterLock {
private let queue = DispatchQueue(label: "com.domain.app.rwLock", attributes: .concurrent)

public func concurrentlyRead<T>(_ block: (() throws -> T)) rethrows -> T {
return try queue.sync {
try block()
}
}

public func exclusivelyWrite(_ block: @escaping (() -> Void)) {
queue.async(flags: .barrier) {
block()
}
}
}

for _ in 0 ..< 5 {
let iterations = 1000
let phoneBook = PhoneBook(.threadSafe)

let concurrentTestQueue = DispatchQueue(label: "com.PhoneBookTest.Queue", attributes: .concurrent)
for _ in 0..<iterations {
let person = Person.uniquePerson()
concurrentTestQueue.async {
phoneBook.addPerson(person)
}
}

concurrentTestQueue.async(flags: .barrier) {
print(phoneBook.count)
}
}

Personally, I'd be inclined to take it a step further and

  • move the synchronization into a generic class; and
  • change the model to be an array of Person object, so that:
    • The model supports multiple people with the same or phone number; and
    • You can use value types if you want.

For example:

public struct Person {
public let name: String
public let phoneNumber: String

public static func uniquePerson() -> Person {
return Person(name: UUID().uuidString, phoneNumber: UUID().uuidString)
}
}

public struct PhoneBook {

private var synchronizedPeople = Synchronized([Person]())

public func people(name: String? = nil, phone: String? = nil) -> [Person]? {
return synchronizedPeople.value.filter {
(name == nil || $0.name == name) && (phone == nil || $0.phoneNumber == phone)
}
}

public func append(_ person: Person) {
synchronizedPeople.writer { people in
people.append(person)
}
}

public var count: Int {
return synchronizedPeople.reader { $0.count }
}
}

/// A structure to provide thread-safe access to some underlying object using reader-writer pattern.

public class Synchronized<T> {
/// Private value. Use `public` `value` computed property (or `reader` and `writer` methods)
/// for safe, thread-safe access to this underlying value.

private var _value: T

/// Private reader-write synchronization queue

private let queue = DispatchQueue(label: Bundle.main.bundleIdentifier! + ".synchronized", qos: .default, attributes: .concurrent)

/// Create `Synchronized` object
///
/// - Parameter value: The initial value to be synchronized.

public init(_ value: T) {
_value = value
}

/// A threadsafe variable to set and get the underlying object, as a convenience when higher level synchronization is not needed

public var value: T {
get { reader { $0 } }
set { writer { $0 = newValue } }
}

/// A "reader" method to allow thread-safe, read-only concurrent access to the underlying object.
///
/// - Warning: If the underlying object is a reference type, you are responsible for making sure you
/// do not mutating anything. If you stick with value types (`struct` or primitive types),
/// this will be enforced for you.

public func reader<U>(_ block: (T) throws -> U) rethrows -> U {
return try queue.sync { try block(_value) }
}

/// A "writer" method to allow thread-safe write with barrier to the underlying object

func writer(_ block: @escaping (inout T) -> Void) {
queue.async(flags: .barrier) {
block(&self._value)
}
}
}

EXC_BAD_ACCESS when initializing Dictionary of CurrentValueSubject in Swift

Swift Dictionary is not thread-safe.
You need to make sure it is being accessed from only one thread (i.e queue) or using locks.

EDIT - another solution suggested by @Bogdan the question writer is to make the class an actor class which the concurrency safety is taken care of by the compiler!

By dispatching to a global queue, you increase the chance that two threads will try and write into the dictionary “at the same time” which probably causes the crash

Take a look at these examples.
How to implement a Thread Safe HashTable (PhoneBook) Data Structure in Swift?

https://github.com/iThink32/Thread-Safe-Dictionary/blob/main/ThreadSafeDictionary.swift

Flipping within a TableCell - like utility application

Firstly, and it's probably just a personal taste, but your code is a bit untidy - there's nothing wrong with a few linebreaks you know ;)

About your problem, I would suggest you use two views within the cell - one will be the cell's default contentView, and the other a custom view that you create yourself.

Then you can write some code into your table view controller that will respond to the info button press. That method should decide which table cell it'll be flipping, using the indexpath, and then carry out the standard view flip using either core animation or a simplistic UIView animation with a flip transition.

It's important to know that each tableViewCell doesn't have a controller for itself by default (you can create one if you want), so when you're dealing with table view cells, most of the things you're interested in will be dealt with by the table view controller.

Things look up:

  • UIView animations

    • +beginAnimations:forContext:
    • +setAnimationTransition:forView:cache:
  • Custom Table Cell View's

    • How table cells are drawn


Related Topics



Leave a reply



Submit