From 39824aff5d4b6ef56283374cc63a486f0e52aaf6 Mon Sep 17 00:00:00 2001 From: Tom Lokhorst Date: Mon, 29 Apr 2019 23:58:19 +0200 Subject: [PATCH 1/2] Refactor internals a bit --- Sources/Promissum/Promise.swift | 18 +++++++++++--- Sources/Promissum/PromiseSource.swift | 36 ++++++++------------------- 2 files changed, 25 insertions(+), 29 deletions(-) diff --git a/Sources/Promissum/Promise.swift b/Sources/Promissum/Promise.swift index 85cde56..3a4ed0a 100644 --- a/Sources/Promissum/Promise.swift +++ b/Sources/Promissum/Promise.swift @@ -99,14 +99,14 @@ public class Promise where Error: Swift.Error { /// /// Example: `Promise(value: 42)` public init(value: Value) { - self.source = PromiseSource(value: value) + self.source = PromiseSource(state: .resolved(value), dispatchKey: DispatchSpecificKey(), dispatchMethod: .unspecified, warnUnresolvedDeinit: false) } /// Initialize a rejected Promise with an error. /// /// Example: `Promise(error: MyError(message: "Oops"))` public init(error: Error) { - self.source = PromiseSource(error: error) + self.source = PromiseSource(state: .rejected(error), dispatchKey: DispatchSpecificKey(), dispatchMethod: .unspecified, warnUnresolvedDeinit: false) } internal init(source: PromiseSource) { @@ -300,7 +300,17 @@ public class Promise where Error: Swift.Error { warnUnresolvedDeinit: true ) - source.addOrCallResultHandler(resultSource.resolveResult) + let handler: (Result) -> Void = { result in + switch result { + case .success(let value): + resultSource.resolve(value) + + case .failure(let error): + resultSource.reject(error) + } + } + + source.addOrCallResultHandler(handler) return resultSource.promise } @@ -478,7 +488,7 @@ public class Promise where Error: Swift.Error { } return source.promise - } + } } } diff --git a/Sources/Promissum/PromiseSource.swift b/Sources/Promissum/PromiseSource.swift index 76960a3..fec3f42 100644 --- a/Sources/Promissum/PromiseSource.swift +++ b/Sources/Promissum/PromiseSource.swift @@ -79,16 +79,6 @@ public class PromiseSource where Error: Swift.Error { /// Print a warning on deinit of an unresolved PromiseSource public var warnUnresolvedDeinit: Bool - // MARK: Initializers & deinit - - internal convenience init(value: Value) { - self.init(state: .resolved(value), dispatchKey: DispatchSpecificKey(), dispatchMethod: .unspecified, warnUnresolvedDeinit: false) - } - - internal convenience init(error: Error) { - self.init(state: .rejected(error), dispatchKey: DispatchSpecificKey(), dispatchMethod: .unspecified, warnUnresolvedDeinit: false) - } - /// Initialize a new Unresolved PromiseSource /// /// - parameter warnUnresolvedDeinit: Print a warning on deinit of an unresolved PromiseSource @@ -137,7 +127,9 @@ public class PromiseSource where Error: Swift.Error { /// /// When called on a PromiseSource that is already Resolved or Rejected, the call is ignored. public func resolve(_ value: Value) { - resolveResult(.success(value)) + if let action = internalState.resolve(with: .success(value)) { + callHandlers(action.handlers, with: action.result, dispatchKey: dispatchKey, dispatchMethod: dispatchMethod) + } } @@ -145,12 +137,9 @@ public class PromiseSource where Error: Swift.Error { /// /// When called on a PromiseSource that is already Resolved or Rejected, the call is ignored. public func reject(_ error: Error) { - resolveResult(.failure(error)) - } - - internal func resolveResult(_ result: Result) { - let action = internalState.resolve(with: result) - callHandlers(action.handlers, with: action.result, dispatchKey: dispatchKey, dispatchMethod: dispatchMethod) + if let action = internalState.resolve(with: .failure(error)) { + callHandlers(action.handlers, with: action.result, dispatchKey: dispatchKey, dispatchMethod: dispatchMethod) + } } // MARK: Adding result handlers @@ -185,21 +174,19 @@ extension PromiseSource { return state } - internal func resolve(with result: Result) -> PromiseSourceAction { + internal func resolve(with result: Result) -> PromiseSourceAction? { lock.lock(); defer { lock.unlock() } - let handlersToExecute: [ResultHandler] - switch state { case .unresolved: state = result.state - handlersToExecute = handlers + let handlersToExecute = handlers handlers = [] + + return PromiseSourceAction(result: result, handlers: handlersToExecute) default: - handlersToExecute = [] + return nil } - - return PromiseSourceAction(result: result, handlers: handlersToExecute) } internal func addHandler(_ handler: @escaping ResultHandler) -> PromiseSourceAction? { @@ -240,7 +227,6 @@ internal func callHandlers(_ handlers: [(T) -> Void], with value: T, dispatch } case .synchronous: - handler(value) case .queue(let targetQueue): From 3f8a3fad6e812be4411463d05a99cc22afb0a847 Mon Sep 17 00:00:00 2001 From: Tom Lokhorst Date: Tue, 30 Apr 2019 00:10:31 +0200 Subject: [PATCH 2/2] Use dispatch queue instead of NSLock for threadsafe state --- Sources/Promissum/PromiseSource.swift | 57 ++++++++++++++------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/Sources/Promissum/PromiseSource.swift b/Sources/Promissum/PromiseSource.swift index fec3f42..d881dd0 100644 --- a/Sources/Promissum/PromiseSource.swift +++ b/Sources/Promissum/PromiseSource.swift @@ -160,7 +160,7 @@ extension PromiseSource { } fileprivate class PromiseSourceState { - private let lock = NSLock() + private let queue = DispatchQueue(label: "PromiseSource internal state queue") private var state: State private var handlers: [ResultHandler] = [] @@ -169,42 +169,43 @@ extension PromiseSource { } internal func readState() -> State { - lock.lock(); defer { lock.unlock() } - - return state + return queue.sync { + return state + } } internal func resolve(with result: Result) -> PromiseSourceAction? { - lock.lock(); defer { lock.unlock() } + return queue.sync { + switch state { + case .unresolved: + state = result.state + let handlersToExecute = handlers + handlers = [] - switch state { - case .unresolved: - state = result.state - let handlersToExecute = handlers - handlers = [] + return PromiseSourceAction(result: result, handlers: handlersToExecute) - return PromiseSourceAction(result: result, handlers: handlersToExecute) - default: - return nil + default: + return nil + } } } internal func addHandler(_ handler: @escaping ResultHandler) -> PromiseSourceAction? { - lock.lock(); defer { lock.unlock() } - - switch state { - case .unresolved: - // Save handler for later - handlers.append(handler) - return nil - - case .resolved(let value): - // Value is already available, call handler immediately - return PromiseSourceAction(result: .success(value), handlers: [handler]) - - case .rejected(let error): - // Error is already available, call handler immediately - return PromiseSourceAction(result: .failure(error), handlers: [handler]) + return queue.sync { + switch state { + case .unresolved: + // Save handler for later + handlers.append(handler) + return nil + + case .resolved(let value): + // Value is already available, call handler immediately + return PromiseSourceAction(result: .success(value), handlers: [handler]) + + case .rejected(let error): + // Error is already available, call handler immediately + return PromiseSourceAction(result: .failure(error), handlers: [handler]) + } } } }