From 83b9b96cd56a467fe21c833142046d394cf267b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Tue, 18 Mar 2025 15:50:40 +0100 Subject: [PATCH 1/4] fix(SimpleResolver): Function loadOrResolve should be atomically executed in the serial queue --- Sources/InfomaniakDI/SimpleResolver.swift | 58 ++++++++++++++--------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/Sources/InfomaniakDI/SimpleResolver.swift b/Sources/InfomaniakDI/SimpleResolver.swift index 5e30a2f..c60fafa 100644 --- a/Sources/InfomaniakDI/SimpleResolver.swift +++ b/Sources/InfomaniakDI/SimpleResolver.swift @@ -65,6 +65,7 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug enum ErrorDomain: Error { case factoryMissing(identifier: String) case typeMissmatch(expected: String, got: String) + case unknown } /// One singleton to rule them all @@ -99,36 +100,49 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug resolver: SimpleResolvable) throws -> Service { let serviceIdentifier = buildIdentifier(type: type, forIdentifier: customIdentifier) - // load form store - var fetchedService: Any? + var resolvedService: Service? + var resolveError: Error? queue.sync { - fetchedService = store[serviceIdentifier] - } - if let service = fetchedService as? Service { - return service + do { + resolvedService = try loadOrResolve( + serviceIdentifier: serviceIdentifier, + factoryParameters: factoryParameters, + resolver: resolver + ) + } catch { + resolveError = error + } } - // load service from factory - var factory: Factoryable? - queue.sync { - factory = factories[serviceIdentifier] - } - guard let factory = factory else { - throw ErrorDomain.factoryMissing(identifier: serviceIdentifier) + guard let resolvedService else { + guard let resolveError else { + throw ErrorDomain.unknown + } + throw resolveError } - // Apply factory closure - let builtType = try factory.build(factoryParameters: factoryParameters, resolver: resolver) - guard let service = builtType as? Service else { - throw ErrorDomain.typeMissmatch(expected: "\(Service.Type.self)", got: "\(builtType.self)") - } + return resolvedService + } + + private func loadOrResolve(serviceIdentifier: String, + factoryParameters: [String: Any]?, + resolver: SimpleResolvable) throws -> Service { + if let fetchedObject = store[serviceIdentifier], + let fetchedService = fetchedObject as? Service { + return fetchedService + } else { + guard let factory = factories[serviceIdentifier] else { + throw ErrorDomain.factoryMissing(identifier: serviceIdentifier) + } + + let builtType = try factory.build(factoryParameters: factoryParameters, resolver: resolver) + guard let service = builtType as? Service else { + throw ErrorDomain.typeMissmatch(expected: "\(Service.Type.self)", got: "\(builtType.self)") + } - // keep in store built object for later - queue.sync { store[serviceIdentifier] = service + return service } - - return service } // MARK: internal From f56c68bd79c9ac2165f4f1b8217cecaccd990e7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Tue, 18 Mar 2025 16:27:41 +0100 Subject: [PATCH 2/4] fix(SimpleResolver): Solve a re-entry issue with serial queue execution --- Sources/InfomaniakDI/SimpleResolver.swift | 26 +++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/Sources/InfomaniakDI/SimpleResolver.swift b/Sources/InfomaniakDI/SimpleResolver.swift index c60fafa..0d3fd5e 100644 --- a/Sources/InfomaniakDI/SimpleResolver.swift +++ b/Sources/InfomaniakDI/SimpleResolver.swift @@ -77,8 +77,14 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug /// Resolved object collection var store = [String: Any]() + private var queueSpecificKey: DispatchSpecificKey = DispatchSpecificKey() + /// A serial queue for thread safety - private let queue = DispatchQueue(label: "com.infomaniakDI.resolver") + private lazy var queue: DispatchQueue = { + let serialQueue = DispatchQueue(label: "com.infomaniakDI.resolver") + serialQueue.setSpecific(key: self.queueSpecificKey, value: serialQueue) + return serialQueue + }() // MARK: SimpleStorable @@ -100,6 +106,14 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug resolver: SimpleResolvable) throws -> Service { let serviceIdentifier = buildIdentifier(type: type, forIdentifier: customIdentifier) + guard notOnInternalQueue else { + return try loadOrResolve( + serviceIdentifier: serviceIdentifier, + factoryParameters: factoryParameters, + resolver: resolver + ) + } + var resolvedService: Service? var resolveError: Error? queue.sync { @@ -128,7 +142,7 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug factoryParameters: [String: Any]?, resolver: SimpleResolvable) throws -> Service { if let fetchedObject = store[serviceIdentifier], - let fetchedService = fetchedObject as? Service { + let fetchedService = fetchedObject as? Service { return fetchedService } else { guard let factory = factories[serviceIdentifier] else { @@ -145,6 +159,14 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug } } + private var notOnInternalQueue: Bool { + if DispatchQueue.getSpecific(key: queueSpecificKey) != nil { + return false + } else { + return true + } + } + // MARK: internal func buildIdentifier(type: Any.Type, From 2cbbedd6fc9f177c7115603298775bbf698db091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 19 Mar 2025 07:26:50 +0100 Subject: [PATCH 3/4] refactor(SimpleResolver): Changed locking to a simpler and more elegant recursive lock --- Sources/InfomaniakDI/SimpleResolver.swift | 93 +++++++---------------- 1 file changed, 27 insertions(+), 66 deletions(-) diff --git a/Sources/InfomaniakDI/SimpleResolver.swift b/Sources/InfomaniakDI/SimpleResolver.swift index 0d3fd5e..9c4192b 100644 --- a/Sources/InfomaniakDI/SimpleResolver.swift +++ b/Sources/InfomaniakDI/SimpleResolver.swift @@ -49,17 +49,18 @@ public protocol SimpleStorable: Sendable { /// A minimalist DI solution /// Once initiated, stores types as long as the app lives public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebugStringConvertible, @unchecked Sendable { + private let recursiveLock = NSRecursiveLock() + public var debugDescription: String { - var buffer: String! - queue.sync { - buffer = """ - <\(type(of: self)):\(Unmanaged.passUnretained(self).toOpaque()) - \(factories.count) factories and \(store.count) stored types - factories: \(factories) - store: \(store)> - """ - } - return buffer + recursiveLock.lock() + defer { recursiveLock.unlock() } + + return """ + <\(type(of: self)):\(Unmanaged.passUnretained(self).toOpaque()) + \(factories.count) factories and \(store.count) stored types + factories: \(factories) + store: \(store)> + """ } enum ErrorDomain: Error { @@ -77,25 +78,16 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug /// Resolved object collection var store = [String: Any]() - private var queueSpecificKey: DispatchSpecificKey = DispatchSpecificKey() - - /// A serial queue for thread safety - private lazy var queue: DispatchQueue = { - let serialQueue = DispatchQueue(label: "com.infomaniakDI.resolver") - serialQueue.setSpecific(key: self.queueSpecificKey, value: serialQueue) - return serialQueue - }() - // MARK: SimpleStorable public func store(factory: Factoryable, forCustomTypeIdentifier customIdentifier: String? = nil) { - let type = factory.type + recursiveLock.lock() + defer { recursiveLock.unlock() } + let type = factory.type let identifier = buildIdentifier(type: type, forIdentifier: customIdentifier) - queue.sync { - factories[identifier] = factory - } + factories[identifier] = factory } // MARK: SimpleResolvable @@ -104,38 +96,15 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug forCustomTypeIdentifier customIdentifier: String?, factoryParameters: [String: Any]? = nil, resolver: SimpleResolvable) throws -> Service { - let serviceIdentifier = buildIdentifier(type: type, forIdentifier: customIdentifier) - - guard notOnInternalQueue else { - return try loadOrResolve( - serviceIdentifier: serviceIdentifier, - factoryParameters: factoryParameters, - resolver: resolver - ) - } - - var resolvedService: Service? - var resolveError: Error? - queue.sync { - do { - resolvedService = try loadOrResolve( - serviceIdentifier: serviceIdentifier, - factoryParameters: factoryParameters, - resolver: resolver - ) - } catch { - resolveError = error - } - } - - guard let resolvedService else { - guard let resolveError else { - throw ErrorDomain.unknown - } - throw resolveError - } + recursiveLock.lock() + defer { recursiveLock.unlock() } - return resolvedService + let serviceIdentifier = buildIdentifier(type: type, forIdentifier: customIdentifier) + return try loadOrResolve( + serviceIdentifier: serviceIdentifier, + factoryParameters: factoryParameters, + resolver: resolver + ) } private func loadOrResolve(serviceIdentifier: String, @@ -159,14 +128,6 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug } } - private var notOnInternalQueue: Bool { - if DispatchQueue.getSpecific(key: queueSpecificKey) != nil { - return false - } else { - return true - } - } - // MARK: internal func buildIdentifier(type: Any.Type, @@ -181,9 +142,9 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug // MARK: testing func removeAll() { - queue.sync { - factories.removeAll() - store.removeAll() - } + recursiveLock.lock() + factories.removeAll() + store.removeAll() + recursiveLock.unlock() } } From ba2c0d8daf2a527c494cb7332f9d05d28f0db1b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Coye=20de=20Brune=CC=81lis?= Date: Wed, 19 Mar 2025 07:44:48 +0100 Subject: [PATCH 4/4] refactor(SimpleResolver): Removed unused unknown error --- Sources/InfomaniakDI/SimpleResolver.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/InfomaniakDI/SimpleResolver.swift b/Sources/InfomaniakDI/SimpleResolver.swift index 9c4192b..28d25d7 100644 --- a/Sources/InfomaniakDI/SimpleResolver.swift +++ b/Sources/InfomaniakDI/SimpleResolver.swift @@ -66,7 +66,6 @@ public final class SimpleResolver: SimpleResolvable, SimpleStorable, CustomDebug enum ErrorDomain: Error { case factoryMissing(identifier: String) case typeMissmatch(expected: String, got: String) - case unknown } /// One singleton to rule them all