From 69d12a61e0dc76a764c462da0632045915fbf0b3 Mon Sep 17 00:00:00 2001 From: Mark Woollard Date: Sat, 19 Dec 2015 17:22:25 +0000 Subject: [PATCH 1/6] Add thread safety Add locking around resolution and dictionary modification to ensure thread safety --- Dip/Dip/Dip.swift | 79 +++++++++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 26 deletions(-) diff --git a/Dip/Dip/Dip.swift b/Dip/Dip/Dip.swift index 83eced2..29bb343 100644 --- a/Dip/Dip/Dip.swift +++ b/Dip/Dip/Dip.swift @@ -57,6 +57,26 @@ public final class DependencyContainer { configBlock(self) } + // MARK: - Thread safety + func threadSafe(closure: () -> Void) { + + objc_sync_enter(self) + defer { + objc_sync_exit(self) + } + closure() + + } + + func threadSafe(closure: () -> T) -> T { + + objc_sync_enter(self) + defer { + objc_sync_exit(self) + } + return closure() + + } // MARK: - Reset all dependencies /** @@ -74,9 +94,10 @@ public final class DependencyContainer { */ public func remove(definition: DefinitionOf, forTag tag: Tag? = nil) { let key = DefinitionKey(protocolType: T.self, factoryType: F.self, associatedTag: tag) - definitions[key] = nil + threadSafe { + self.definitions[key] = nil + } } - // MARK: Register dependencies /** @@ -125,7 +146,9 @@ public final class DependencyContainer { public func registerFactory(tag tag: Tag? = nil, scope: ComponentScope, factory: F) -> DefinitionOf { let key = DefinitionKey(protocolType: T.self, factoryType: F.self, associatedTag: tag) let definition = DefinitionOf(factory: factory, scope: scope) - definitions[key] = definition + threadSafe { + self.definitions[key] = definition + } return definition } @@ -138,7 +161,9 @@ public final class DependencyContainer { */ public func register(definition: DefinitionOf, forTag tag: Tag? = nil) { let key = DefinitionKey(protocolType: T.self, factoryType: F.self, associatedTag: tag) - definitions[key] = definition + threadSafe { + self.definitions[key] = definition + } } // MARK: Resolve dependencies @@ -190,10 +215,12 @@ public final class DependencyContainer { let key = DefinitionKey(protocolType: T.self, factoryType: F.self, associatedTag: tag) let nilTagKey = tag.map { _ in DefinitionKey(protocolType: T.self, factoryType: F.self, associatedTag: nil) } - guard let definition = (self.definitions[key] ?? self.definitions[nilTagKey]) as? DefinitionOf else { - throw DipError.DefinitionNotFound(key) + guard let definition = (threadSafe { + return (self.definitions[key] ?? self.definitions[nilTagKey]) + }) as? DefinitionOf else { + throw DipError.DefinitionNotFound(key) } - + let usingKey: DefinitionKey? = definition.scope == .ObjectGraph ? key : nil return _resolve(tag, key: usingKey, definition: definition, builder: builder) } @@ -201,30 +228,30 @@ public final class DependencyContainer { /// Actually resolve dependency private func _resolve(tag: Tag? = nil, key: DefinitionKey?, definition: DefinitionOf, builder: F->T) -> T { - return resolvedInstances.resolve { - - if let previouslyResolved: T = resolvedInstances.previouslyResolved(key, definition: definition) { - return previouslyResolved - } - else { - let resolvedInstance = builder(definition.factory) + return threadSafe { + return self.resolvedInstances.resolve { - //when builder calls factory it will in turn resolve sub-dependencies (if there are any) - //when it returns instance that we try to resolve here can be already resolved - //so we return it, throwing away instance created by previous call to builder - if let previouslyResolved: T = resolvedInstances.previouslyResolved(key, definition: definition) { + if let previouslyResolved: T = self.resolvedInstances.previouslyResolved(key, definition: definition) { return previouslyResolved } - - resolvedInstances.storeResolvedInstance(resolvedInstance, forKey: key) - definition.resolvedInstance = resolvedInstance - definition.resolveDependenciesBlock?(self, resolvedInstance) - - return resolvedInstance + else { + let resolvedInstance = builder(definition.factory) + + //when builder calls factory it will in turn resolve sub-dependencies (if there are any) + //when it returns instance that we try to resolve here can be already resolved + //so we return it, throwing away instance created by previous call to builder + if let previouslyResolved: T = self.resolvedInstances.previouslyResolved(key, definition: definition) { + return previouslyResolved + } + + self.resolvedInstances.storeResolvedInstance(resolvedInstance, forKey: key) + definition.resolvedInstance = resolvedInstance + definition.resolveDependenciesBlock?(self, resolvedInstance) + + return resolvedInstance + } } - } - } // MARK: - Private From 6e18211edb711167437c55be1a8ec1fa84baef6c Mon Sep 17 00:00:00 2001 From: Mark Woollard Date: Sat, 19 Dec 2015 22:04:06 +0000 Subject: [PATCH 2/6] Switch lock to NSRecursiveLock and make thread safety configurable NSRecursiveLock is quicker than objc_sync_enter/exit so switch to it Add init parameter to disable thread safety, default is enabled --- Dip/Dip/Dip.swift | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Dip/Dip/Dip.swift b/Dip/Dip/Dip.swift index 29bb343..1c1d57d 100644 --- a/Dip/Dip/Dip.swift +++ b/Dip/Dip/Dip.swift @@ -41,28 +41,33 @@ public final class DependencyContainer { } var definitions = [DefinitionKey : Definition]() + var lock: NSRecursiveLock? /** Designated initializer for a DependencyContainer - parameter configBlock: A configuration block in which you typically put all you `register` calls. - + - parameter isThreadSafe: If true instance is thread safe, if false not thread safe but + slight performance gain. Default is to be thread safe. - note: The `configBlock` is simply called at the end of the `init` to let you configure everything. It is only present for convenience to have a cleaner syntax when declaring and initializing your `DependencyContainer` instances. - returns: A new DependencyContainer. */ - public init(@noescape configBlock: (DependencyContainer->()) = { _ in }) { + public init(isThreadSafe: Bool = true, @noescape configBlock: (DependencyContainer->()) = { _ in }) { + if isThreadSafe { + lock = NSRecursiveLock() + } configBlock(self) } // MARK: - Thread safety func threadSafe(closure: () -> Void) { - objc_sync_enter(self) + lock?.lock() defer { - objc_sync_exit(self) + lock?.unlock() } closure() @@ -70,9 +75,9 @@ public final class DependencyContainer { func threadSafe(closure: () -> T) -> T { - objc_sync_enter(self) + lock?.lock() defer { - objc_sync_exit(self) + lock?.unlock() } return closure() From ff41e14e3c4433e80a55a5d6db68369c8897cd79 Mon Sep 17 00:00:00 2001 From: Mark Woollard Date: Sat, 19 Dec 2015 22:04:27 +0000 Subject: [PATCH 3/6] Add unit tests for thread safety and timing --- Dip/Dip.xcodeproj/project.pbxproj | 8 ++ Dip/DipTests/ComponentScopeTests.swift | 28 ++--- Dip/DipTests/DipTests.swift | 17 ++- Dip/DipTests/ThreadSafetyTests.swift | 150 +++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 16 deletions(-) create mode 100644 Dip/DipTests/ThreadSafetyTests.swift diff --git a/Dip/Dip.xcodeproj/project.pbxproj b/Dip/Dip.xcodeproj/project.pbxproj index bedb545..1f63ab3 100644 --- a/Dip/Dip.xcodeproj/project.pbxproj +++ b/Dip/Dip.xcodeproj/project.pbxproj @@ -38,6 +38,9 @@ 0919F4EC1C16419500DC3B10 /* DefinitionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0919F4CF1C16417000DC3B10 /* DefinitionTests.swift */; }; 0919F4ED1C16419500DC3B10 /* RuntimeArgumentsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0919F4D21C16417000DC3B10 /* RuntimeArgumentsTests.swift */; }; 0919F4EE1C16419500DC3B10 /* ComponentScopeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0919F4CE1C16417000DC3B10 /* ComponentScopeTests.swift */; }; + 2C15B9511C25F01200EA3486 /* ThreadSafetyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C15B94F1C25EF7800EA3486 /* ThreadSafetyTests.swift */; }; + 2C15B9521C25F01300EA3486 /* ThreadSafetyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C15B94F1C25EF7800EA3486 /* ThreadSafetyTests.swift */; }; + 2C15B9531C25F01500EA3486 /* ThreadSafetyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2C15B94F1C25EF7800EA3486 /* ThreadSafetyTests.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -82,6 +85,7 @@ 0919F4D01C16417000DC3B10 /* DipTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DipTests.swift; sourceTree = ""; }; 0919F4D11C16417000DC3B10 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; 0919F4D21C16417000DC3B10 /* RuntimeArgumentsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RuntimeArgumentsTests.swift; sourceTree = ""; }; + 2C15B94F1C25EF7800EA3486 /* ThreadSafetyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ThreadSafetyTests.swift; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -160,6 +164,7 @@ 0919F4D21C16417000DC3B10 /* RuntimeArgumentsTests.swift */, 0919F4CE1C16417000DC3B10 /* ComponentScopeTests.swift */, 0919F4D11C16417000DC3B10 /* Info.plist */, + 2C15B94F1C25EF7800EA3486 /* ThreadSafetyTests.swift */, ); path = DipTests; sourceTree = ""; @@ -477,6 +482,7 @@ buildActionMask = 2147483647; files = ( 0919F4E61C16419300DC3B10 /* ComponentScopeTests.swift in Sources */, + 2C15B9511C25F01200EA3486 /* ThreadSafetyTests.swift in Sources */, 0919F4E41C16419300DC3B10 /* DefinitionTests.swift in Sources */, 0919F4E31C16419300DC3B10 /* DipTests.swift in Sources */, 0919F4E51C16419300DC3B10 /* RuntimeArgumentsTests.swift in Sources */, @@ -498,6 +504,7 @@ buildActionMask = 2147483647; files = ( 0919F4EA1C16419400DC3B10 /* ComponentScopeTests.swift in Sources */, + 2C15B9521C25F01300EA3486 /* ThreadSafetyTests.swift in Sources */, 0919F4E81C16419400DC3B10 /* DefinitionTests.swift in Sources */, 0919F4E71C16419400DC3B10 /* DipTests.swift in Sources */, 0919F4E91C16419400DC3B10 /* RuntimeArgumentsTests.swift in Sources */, @@ -519,6 +526,7 @@ buildActionMask = 2147483647; files = ( 0919F4EE1C16419500DC3B10 /* ComponentScopeTests.swift in Sources */, + 2C15B9531C25F01500EA3486 /* ThreadSafetyTests.swift in Sources */, 0919F4EC1C16419500DC3B10 /* DefinitionTests.swift in Sources */, 0919F4EB1C16419500DC3B10 /* DipTests.swift in Sources */, 0919F4ED1C16419500DC3B10 /* RuntimeArgumentsTests.swift in Sources */, diff --git a/Dip/DipTests/ComponentScopeTests.swift b/Dip/DipTests/ComponentScopeTests.swift index 486ec9f..000c620 100644 --- a/Dip/DipTests/ComponentScopeTests.swift +++ b/Dip/DipTests/ComponentScopeTests.swift @@ -25,6 +25,20 @@ import XCTest @testable import Dip +class Server { + weak var client: Client? + + init() {} +} + +class Client { + var server: Server + + init(server: Server) { + self.server = server + } +} + class ComponentScopeTests: XCTestCase { let container = DependencyContainer() @@ -74,20 +88,6 @@ class ComponentScopeTests: XCTestCase { XCTAssertTrue((service1 as! ServiceImp1) === (service2 as! ServiceImp1)) } - class Server { - weak var client: Client? - - init() {} - } - - class Client { - var server: Server - - init(server: Server) { - self.server = server - } - } - func testThatItReusesInstanceInObjectGraphScopeDuringResolve() { //given container.register(.ObjectGraph) { Client(server: try! self.container.resolve()) as Client } diff --git a/Dip/DipTests/DipTests.swift b/Dip/DipTests/DipTests.swift index 38c8e14..c25bd69 100644 --- a/Dip/DipTests/DipTests.swift +++ b/Dip/DipTests/DipTests.swift @@ -35,9 +35,22 @@ extension Service { } } -class ServiceImp1: Service {} +internal func ==(lhs: HashableService, rhs: HashableService) -> Bool { + return lhs === rhs +} + +class HashableService : Service, Hashable { + internal var hashValue: Int { get { + return unsafeAddressOf(self).hashValue + } + } +} + +class ServiceImp1: HashableService { +} -class ServiceImp2: Service {} +class ServiceImp2: HashableService { +} class DipTests: XCTestCase { diff --git a/Dip/DipTests/ThreadSafetyTests.swift b/Dip/DipTests/ThreadSafetyTests.swift new file mode 100644 index 0000000..07c02ba --- /dev/null +++ b/Dip/DipTests/ThreadSafetyTests.swift @@ -0,0 +1,150 @@ +// +// Dip +// +// Copyright (c) 2015 Olivier Halligon +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +import XCTest +@testable import Dip + +class ThreadSafetyTests: XCTestCase { + + let container = DependencyContainer() + + override func setUp() { + super.setUp() + container.reset() + } + + func testSingletonThreadSafety() { + + container.register(.Singleton) { ServiceImp1() as Service } + + let queue = NSOperationQueue() + let lock = NSRecursiveLock() + var resultSet = Set() + + for _ in 1...100 { + queue.addOperationWithBlock { + let serviceInstance = try! self.container.resolve() as Service + + lock.lock() + resultSet.insert(serviceInstance as! ServiceImp1) + lock.unlock() + } + } + + queue.waitUntilAllOperationsAreFinished() + + XCTAssertEqual(resultSet.count, 1) + } + + func testFactoryThreadSafety() { + + container.register() { ServiceImp1() as Service } + + let queue = NSOperationQueue() + let lock = NSRecursiveLock() + var resultSet = Set() + + for _ in 1...100 { + queue.addOperationWithBlock { + let serviceInstance = try! self.container.resolve() as Service + + lock.lock() + resultSet.insert(serviceInstance as! ServiceImp1) + lock.unlock() + } + } + + queue.waitUntilAllOperationsAreFinished() + + XCTAssertEqual(resultSet.count, 100) + } + + func testCircularReferenceThreadSafety() { + //given + container.register(.ObjectGraph) { Client(server: try! self.container.resolve()) as Client } + + container.register(.ObjectGraph) { Server() as Server }.resolveDependencies { container, server in + server.client = try! container.resolve() as Client + } + + let queue = NSOperationQueue() + + for _ in 1...100 { + queue.addOperationWithBlock { + //when + let client = try! self.container.resolve() as Client + + //then + let server = client.server + XCTAssertTrue(server.client === client) + } + } + + queue.waitUntilAllOperationsAreFinished() + } + + func testThreadSafetyPerformance() { + container.register() { ServiceImp1() as Service } + + measureBlock() { + for _ in 1...10000 { + let _ = try! self.container.resolve() as Service + } + } + } + + func testNoThreadSafetyPerformance() { + let unsafeContainer = DependencyContainer(isThreadSafe:false) + unsafeContainer.register() { ServiceImp1() as Service } + + measureBlock() { + for _ in 1...10000 { + let _ = try! unsafeContainer.resolve() as Service + } + } + } + + func testNSRecursiveLockPerformance() { + + let lock = NSRecursiveLock() + + measureBlock() { + for _ in 1...1000000 { + lock.lock() + lock.unlock() + } + } + } + + func testObjcSyncEnterExitPerformance() { + + measureBlock() { + for _ in 1...1000000 { + objc_sync_enter(self) + objc_sync_exit(self) + } + } + + } +} \ No newline at end of file From f04435c32fb91db80968712d462670a4080b75e8 Mon Sep 17 00:00:00 2001 From: Mark Woollard Date: Sun, 20 Dec 2015 08:31:27 +0000 Subject: [PATCH 4/6] Incorporate feedback from @ilyapuchka Removed option to disable thread safety Moved locking for resolve to single lock in public resolve func Added locking in reset func --- Dip/Dip/Dip.swift | 79 ++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/Dip/Dip/Dip.swift b/Dip/Dip/Dip.swift index 1c1d57d..f294d7f 100644 --- a/Dip/Dip/Dip.swift +++ b/Dip/Dip/Dip.swift @@ -41,45 +41,40 @@ public final class DependencyContainer { } var definitions = [DefinitionKey : Definition]() - var lock: NSRecursiveLock? + let lock = NSRecursiveLock() /** Designated initializer for a DependencyContainer - parameter configBlock: A configuration block in which you typically put all you `register` calls. - - parameter isThreadSafe: If true instance is thread safe, if false not thread safe but - slight performance gain. Default is to be thread safe. - - note: The `configBlock` is simply called at the end of the `init` to let you configure everything. + - note: The `configBlock` is simply called at the end of the `init` to let you configure everything. It is only present for convenience to have a cleaner syntax when declaring and initializing your `DependencyContainer` instances. - returns: A new DependencyContainer. */ - public init(isThreadSafe: Bool = true, @noescape configBlock: (DependencyContainer->()) = { _ in }) { - if isThreadSafe { - lock = NSRecursiveLock() - } + public init(@noescape configBlock: (DependencyContainer->()) = { _ in }) { configBlock(self) } // MARK: - Thread safety - func threadSafe(closure: () -> Void) { + private func threadSafe(closure: () -> Void) { - lock?.lock() + lock.lock() defer { - lock?.unlock() + lock.unlock() } closure() } - func threadSafe(closure: () -> T) -> T { + private func threadSafe(closure: () throws -> T) throws -> T { - lock?.lock() + lock.lock() defer { - lock?.unlock() + lock.unlock() } - return closure() + return try closure() } // MARK: - Reset all dependencies @@ -88,7 +83,9 @@ public final class DependencyContainer { Clear all the previously registered dependencies on this container. */ public func reset() { - definitions.removeAll() + threadSafe { + self.definitions.removeAll() + } } /** @@ -220,41 +217,39 @@ public final class DependencyContainer { let key = DefinitionKey(protocolType: T.self, factoryType: F.self, associatedTag: tag) let nilTagKey = tag.map { _ in DefinitionKey(protocolType: T.self, factoryType: F.self, associatedTag: nil) } - guard let definition = (threadSafe { - return (self.definitions[key] ?? self.definitions[nilTagKey]) - }) as? DefinitionOf else { - throw DipError.DefinitionNotFound(key) + return try threadSafe { + guard let definition = (self.definitions[key] ?? self.definitions[nilTagKey]) as? DefinitionOf else { + throw DipError.DefinitionNotFound(key) + } + + let usingKey: DefinitionKey? = definition.scope == .ObjectGraph ? key : nil + return self._resolve(tag, key: usingKey, definition: definition, builder: builder) } - - let usingKey: DefinitionKey? = definition.scope == .ObjectGraph ? key : nil - return _resolve(tag, key: usingKey, definition: definition, builder: builder) } /// Actually resolve dependency private func _resolve(tag: Tag? = nil, key: DefinitionKey?, definition: DefinitionOf, builder: F->T) -> T { - return threadSafe { - return self.resolvedInstances.resolve { + return resolvedInstances.resolve { + + if let previouslyResolved: T = resolvedInstances.previouslyResolved(key, definition: definition) { + return previouslyResolved + } + else { + let resolvedInstance = builder(definition.factory) - if let previouslyResolved: T = self.resolvedInstances.previouslyResolved(key, definition: definition) { + //when builder calls factory it will in turn resolve sub-dependencies (if there are any) + //when it returns instance that we try to resolve here can be already resolved + //so we return it, throwing away instance created by previous call to builder + if let previouslyResolved: T = resolvedInstances.previouslyResolved(key, definition: definition) { return previouslyResolved } - else { - let resolvedInstance = builder(definition.factory) - - //when builder calls factory it will in turn resolve sub-dependencies (if there are any) - //when it returns instance that we try to resolve here can be already resolved - //so we return it, throwing away instance created by previous call to builder - if let previouslyResolved: T = self.resolvedInstances.previouslyResolved(key, definition: definition) { - return previouslyResolved - } - - self.resolvedInstances.storeResolvedInstance(resolvedInstance, forKey: key) - definition.resolvedInstance = resolvedInstance - definition.resolveDependenciesBlock?(self, resolvedInstance) - - return resolvedInstance - } + + resolvedInstances.storeResolvedInstance(resolvedInstance, forKey: key) + definition.resolvedInstance = resolvedInstance + definition.resolveDependenciesBlock?(self, resolvedInstance) + + return resolvedInstance } } } From 0407c2f1875989c9c3686a5d281332e5b026d502 Mon Sep 17 00:00:00 2001 From: Mark Woollard Date: Sun, 20 Dec 2015 11:03:11 +0000 Subject: [PATCH 5/6] Updated thread safety unit tests --- Dip/DipTests/ThreadSafetyTests.swift | 84 +++++++++++----------------- 1 file changed, 33 insertions(+), 51 deletions(-) diff --git a/Dip/DipTests/ThreadSafetyTests.swift b/Dip/DipTests/ThreadSafetyTests.swift index 07c02ba..a401480 100644 --- a/Dip/DipTests/ThreadSafetyTests.swift +++ b/Dip/DipTests/ThreadSafetyTests.swift @@ -36,12 +36,17 @@ class ThreadSafetyTests: XCTestCase { func testSingletonThreadSafety() { - container.register(.Singleton) { ServiceImp1() as Service } - let queue = NSOperationQueue() let lock = NSRecursiveLock() var resultSet = Set() + container.register() { ServiceImp2() as HashableService } + container.register(.Singleton) { ServiceImp1() as Service }.resolveDependencies {_,_ in + queue.addOperationWithBlock{ () -> Void in + let _ = try! self.container.resolve() as HashableService + } + } + for _ in 1...100 { queue.addOperationWithBlock { let serviceInstance = try! self.container.resolve() as Service @@ -59,12 +64,17 @@ class ThreadSafetyTests: XCTestCase { func testFactoryThreadSafety() { - container.register() { ServiceImp1() as Service } - let queue = NSOperationQueue() let lock = NSRecursiveLock() var resultSet = Set() + container.register() { ServiceImp2() as HashableService } + container.register() { ServiceImp1() as Service }.resolveDependencies {_,_ in + queue.addOperationWithBlock{ () -> Void in + let _ = try! self.container.resolve() as HashableService + } + } + for _ in 1...100 { queue.addOperationWithBlock { let serviceInstance = try! self.container.resolve() as Service @@ -81,70 +91,42 @@ class ThreadSafetyTests: XCTestCase { } func testCircularReferenceThreadSafety() { - //given - container.register(.ObjectGraph) { Client(server: try! self.container.resolve()) as Client } + + let queue = NSOperationQueue() + let lock = NSLock() + + container.register() { ServiceImp1() as Service } + + container.register(.ObjectGraph) { + Client(server: try! self.container.resolve()) as Client + } container.register(.ObjectGraph) { Server() as Server }.resolveDependencies { container, server in server.client = try! container.resolve() as Client + queue.addOperationWithBlock { + let _ = try! container.resolve() as Service + } } - let queue = NSOperationQueue() + var results = Array() for _ in 1...100 { queue.addOperationWithBlock { //when let client = try! self.container.resolve() as Client - //then - let server = client.server - XCTAssertTrue(server.client === client) + lock.lock() + results.append(client) + lock.unlock() } } queue.waitUntilAllOperationsAreFinished() - } - - func testThreadSafetyPerformance() { - container.register() { ServiceImp1() as Service } - measureBlock() { - for _ in 1...10000 { - let _ = try! self.container.resolve() as Service - } + for client in results { + let server = client.server + XCTAssertTrue(server.client === client) } } - func testNoThreadSafetyPerformance() { - let unsafeContainer = DependencyContainer(isThreadSafe:false) - unsafeContainer.register() { ServiceImp1() as Service } - - measureBlock() { - for _ in 1...10000 { - let _ = try! unsafeContainer.resolve() as Service - } - } - } - - func testNSRecursiveLockPerformance() { - - let lock = NSRecursiveLock() - - measureBlock() { - for _ in 1...1000000 { - lock.lock() - lock.unlock() - } - } - } - - func testObjcSyncEnterExitPerformance() { - - measureBlock() { - for _ in 1...1000000 { - objc_sync_enter(self) - objc_sync_exit(self) - } - } - - } } \ No newline at end of file From fb89794cae40def1af1bfade0b10aaafa4a406df Mon Sep 17 00:00:00 2001 From: Mark Woollard Date: Sun, 20 Dec 2015 11:24:11 +0000 Subject: [PATCH 6/6] Update thread safety test to check resolving dependency on separate thread --- Dip/DipTests/ThreadSafetyTests.swift | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/Dip/DipTests/ThreadSafetyTests.swift b/Dip/DipTests/ThreadSafetyTests.swift index a401480..49f05ac 100644 --- a/Dip/DipTests/ThreadSafetyTests.swift +++ b/Dip/DipTests/ThreadSafetyTests.swift @@ -40,12 +40,7 @@ class ThreadSafetyTests: XCTestCase { let lock = NSRecursiveLock() var resultSet = Set() - container.register() { ServiceImp2() as HashableService } - container.register(.Singleton) { ServiceImp1() as Service }.resolveDependencies {_,_ in - queue.addOperationWithBlock{ () -> Void in - let _ = try! self.container.resolve() as HashableService - } - } + container.register(.Singleton) { ServiceImp1() as Service } for _ in 1...100 { queue.addOperationWithBlock { @@ -68,12 +63,7 @@ class ThreadSafetyTests: XCTestCase { let lock = NSRecursiveLock() var resultSet = Set() - container.register() { ServiceImp2() as HashableService } - container.register() { ServiceImp1() as Service }.resolveDependencies {_,_ in - queue.addOperationWithBlock{ () -> Void in - let _ = try! self.container.resolve() as HashableService - } - } + container.register() { ServiceImp1() as Service } for _ in 1...100 { queue.addOperationWithBlock { @@ -95,17 +85,16 @@ class ThreadSafetyTests: XCTestCase { let queue = NSOperationQueue() let lock = NSLock() - container.register() { ServiceImp1() as Service } - container.register(.ObjectGraph) { Client(server: try! self.container.resolve()) as Client } container.register(.ObjectGraph) { Server() as Server }.resolveDependencies { container, server in - server.client = try! container.resolve() as Client - queue.addOperationWithBlock { - let _ = try! container.resolve() as Service + var client: Client? + dispatch_sync(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0)) { + client = try! container.resolve() as Client } + server.client = client! } var results = Array()