-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Synchronise access to shared table info
dictionary
#101
Conversation
table info
dictionary
@@ -26,7 +26,7 @@ import Dispatch | |||
|
|||
public class TableInfo { | |||
private var codableMap = [String: (info: TypeInfo, table: Table)]() | |||
private var codableMapLock = DispatchSemaphore(value: 1) | |||
private var codableMapQueue = DispatchQueue(label: "codaleMap.queue", attributes: .concurrent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: typo in queue name
return result | ||
var decodeError: Error? | ||
// Prevent concurrent access to codableMap while writing | ||
do { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to capture decodeError? If you take the do/catch away, the error will be thrown out of queue.sync and this function will exit early (throwing). The logic that follows could then be simplified to:
guard let decodeResult = result else {
throw RequestError...
}
return decodeResult
@@ -35,18 +35,33 @@ public class TableInfo { | |||
|
|||
func getInfo<T: Decodable>(_ idColumn: (name: String, type: SQLDataType.Type), _ tableName: String, _ type: T.Type) throws -> (info: TypeInfo, table: Table) { | |||
let typeString = "\(type)" | |||
if let result = codableMap[typeString] { | |||
var result: (TypeInfo, Table)? = nil | |||
// Read from codableMap when no concurrent write is occuring |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
occurring.
This PR adds synchronization around access to the dictionary used to store table info.
Without this synchronization concurrent access can lead to crashes on Swift 5.