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:
By the way, this simplified
ReaderWriterLock
happens to solves another concern. ThatwriteClosure
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 tonil
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.)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.
Note, I did not just print
nameToPersonMap.count
. This array has been carefully synchronized withinPhoneBook
, 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
}
}
}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 yourPhoneBook
with the.threadSafe
option.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
Swiftui MACos Xcode Style Toolbar
Accessing Self from Instance Properties Which Are Closures
Uisearchcontroller in Navigationitem iOS 11 Apple Way
Swift Tableview Cell Set Accessory Type
Swiftui - How to Use Oncommand with Nsmenuitem on MACos
Compiler Error: Invalid Library File - Corelocation
Adding Nscoding as an Extension
Giving Physics to Tiles of Sktilemapnode in Xcode 8
Accessing Multiple Audio Hardware Outputs/Channels Using Avfoundation and Swift
How to Specify That a Generic Is a Value Type
Scenekit - Get Direction of Camera
How to Convert Nsurl to String in Swift
Uitableviewrowaction with Icon and Text
How to Set Title of Navigation Bar in Swift
Sprite-Kit: Moving an Element in Circular Path
How to Add Initializers in Extensions to Existing Uikit Classes Such as Uicolor