Add thread safety with NSRecursiveLock

CHANGELOG.md

@@ -2,10 +2,15 @@
 
 ## Develop
 
+#### New features
+
 * Added auto-injection feature.  
   [#13](https://github.com/AliSoftware/Dip/pull/13), [@ilyapuchka](https://github.com/ilyapuchka)
 * Factories and `resolveDependencies` blocks of `DefinitionOf` are now allowed to `throw`.  
   [#32](https://github.com/AliSoftware/Dip/pull/13), [@ilyapuchka](https://github.com/ilyapuchka)
+* Thread safety reimplemented with support for recursive methods calls.  
+  [#31](https://github.com/AliSoftware/Dip/pull/31), [@mwoollard](https://github.com/mwoollard)
+
 
 ## 4.0.0
 

Dip/Dip.xcodeproj/project.pbxproj

@@ -45,6 +45,9 @@
 		09873F7A1C1E0252000C02F6 /* AutoInjection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 09873F551C1E0237000C02F6 /* AutoInjection.swift */; };
 		09873F7B1C1E0253000C02F6 /* AutoInjection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 09873F551C1E0237000C02F6 /* AutoInjection.swift */; };
 		09873F7C1C1E0254000C02F6 /* AutoInjection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 09873F551C1E0237000C02F6 /* AutoInjection.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 */
@@ -91,6 +94,7 @@
 		0919F4D21C16417000DC3B10 /* RuntimeArgumentsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RuntimeArgumentsTests.swift; sourceTree = "<group>"; };
 		09873F551C1E0237000C02F6 /* AutoInjection.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AutoInjection.swift; sourceTree = "<group>"; };
 		09873F751C1E0249000C02F6 /* AutoInjectionTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AutoInjectionTests.swift; sourceTree = "<group>"; };
+		2C15B94F1C25EF7800EA3486 /* ThreadSafetyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ThreadSafetyTests.swift; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -170,6 +174,7 @@
 				0919F4D21C16417000DC3B10 /* RuntimeArgumentsTests.swift */,
 				0919F4CE1C16417000DC3B10 /* ComponentScopeTests.swift */,
 				09873F751C1E0249000C02F6 /* AutoInjectionTests.swift */,
+				2C15B94F1C25EF7800EA3486 /* ThreadSafetyTests.swift */,
 				0919F4D11C16417000DC3B10 /* Info.plist */,
 			);
 			path = DipTests;
@@ -489,6 +494,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 */,
@@ -512,6 +518,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 */,
@@ -535,6 +542,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 */,

Dip/Dip/Dip.swift

@@ -41,6 +41,7 @@ public final class DependencyContainer {
   }
 
   var definitions = [DefinitionKey : Definition]()
+  let lock = NSRecursiveLock()
 
   /**
    Designated initializer for a DependencyContainer
@@ -57,6 +58,16 @@ public final class DependencyContainer {
     configBlock(self)
   }
 
+  // MARK: - Thread safety
+
+  private func threadSafe<T>(closure: () throws -> T) rethrows -> T {
+    lock.lock()
+    defer {
+      lock.unlock()
+    }
+    return try closure()
+  }
+
   // MARK: - Removing definitions
 
   /**
@@ -71,18 +82,22 @@ public final class DependencyContainer {
   }
 
   public func remove(definition: Definition, forKey key: DefinitionKey) {
-    definitions[key] = nil
-    if let definition = definition as? AutoInjectedDefinition {
-      removeInjected(definition)
-      removeInjectedWeak(definition)
+    threadSafe {
+      self.definitions[key] = nil
+      if let definition = definition as? AutoInjectedDefinition {
+        self.removeInjected(definition)
+        self.removeInjectedWeak(definition)
+      }
     }
   }
 
   /**
    Clear all the previously registered dependencies on this container.
    */
   public func reset() {
-    definitions.removeAll()
+    threadSafe {
+      self.definitions.removeAll()
+    }
   }
 
   // MARK: Register definitions
@@ -149,11 +164,13 @@ public final class DependencyContainer {
   }
 
   public func register(definition: Definition, forKey key: DefinitionKey) {
-    definitions[key] = definition
+    threadSafe {
+      self.definitions[key] = definition
 
-    if let definition = definition as? AutoInjectedDefinition where key.associatedTag == nil {
-      registerInjected(definition)
-      registerInjectedWeak(definition)
+      if let definition = definition as? AutoInjectedDefinition where key.associatedTag == nil {
+        self.registerInjected(definition)
+        self.registerInjectedWeak(definition)
+      }
     }
   }
 
@@ -206,14 +223,14 @@ 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<T, F> else {
-      let error = DipError.DefinitionNotFound(key)
-      print("\(error)")
-      throw error
+    return try threadSafe {
+      guard let definition = (self.definitions[key] ?? self.definitions[nilTagKey]) as? DefinitionOf<T, F> else {
+          throw DipError.DefinitionNotFound(key)
+      }
+
+      let usingKey: DefinitionKey? = definition.scope == .ObjectGraph ? key : nil
+      return try self._resolve(tag, key: usingKey, definition: definition, builder: builder)
     }
-
-    let usingKey: DefinitionKey? = definition.scope == .ObjectGraph ? key : nil
-    return try _resolve(tag, key: usingKey, definition: definition, builder: builder)
   }
 
   /// Actually resolve dependency
@@ -244,7 +261,6 @@ public final class DependencyContainer {
         return resolvedInstance
       }
     }
-
   }
 
   // MARK: - Private

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 {
 

Dip/DipTests/ThreadSafetyTests.swift

@@ -0,0 +1,135 @@
+//
+// Dip
+//
+// Copyright (c) 2015 Olivier Halligon <olivier@halligon.net>
+//
+// 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 {
+
+  class Server {
+    weak var client: Client?
+
+    init() {}
+  }
+
+  class Client {
+    var server: Server
+
+    init(server: Server) {
+      self.server = server
+    }
+  }
+
+  let container = DependencyContainer()
+
+  override func setUp() {
+    super.setUp()
+    container.reset()
+  }
+
+  func testSingletonThreadSafety() {
+
+    let queue = NSOperationQueue()
+    let lock = NSRecursiveLock()
+    var resultSet = Set<ServiceImp1>()
+
+    container.register(.Singleton) { ServiceImp1() as Service }
+
+    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() {
+
+    let queue = NSOperationQueue()
+    let lock = NSRecursiveLock()
+    var resultSet = Set<ServiceImp1>()
+
+    container.register() { ServiceImp1() as Service }
+
+    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() {
+
+    let queue = NSOperationQueue()
+    let lock = NSLock()
+
+    container.register(.ObjectGraph) {
+      Client(server: try self.container.resolve()) as Client
+    }
+
+    container.register(.ObjectGraph) { Server() as Server }.resolveDependencies { container, server in
+      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<Client>()
+
+    for _ in 1...100 {
+      queue.addOperationWithBlock {
+        //when
+        let client = try! self.container.resolve() as Client
+
+        lock.lock()
+        results.append(client)
+        lock.unlock()
+      }
+    }
+
+    queue.waitUntilAllOperationsAreFinished()
+
+    for client in results {
+      let server = client.server
+      XCTAssertTrue(server.client === client)
+    }
+  }
+
+}

README.md

@@ -214,13 +214,14 @@ You can find more use cases for auto-injection in the Playground available in th
 
 ### Thread safety
 
-_Dip_ does not provide thread safety, so you need to make sure you always call `resolve` method of `DependencyContainer` from the single thread. 
-Otherwise if two threads try to resolve the same type they can get different instances where the same instance is expected.
+`DependencyContainer` is thread safe, you can register and resolve components from different threads. 
+Still we encourage to register components in the main thread early in the application lifecycle to prevent race conditions 
+when you try to resolve component from one thread while it was not yet registered in container by another thread.
 
 ### Errors
 
 The resolve operation is potentially dangerous because you can use the wrong type, factory or a wrong tag. For that reason Dip throws an error
- `DefinitionNotFond(DefinitionKey)` if it failed to resolve type. When calling `resolve` you need to use a `try` operator. 
+ `DefinitionNotFound(DefinitionKey)` if it failed to resolve type. When calling `resolve` you need to use a `try` operator. 
  There are rare use cases where your application can recover from this kind of errors (for example you can register new types 
  when user unlocks some content). In most of the cases you can use `try!` to cause an exception at runtime if error was thrown
   or `try?` if it is appropriate in your case to have `nil`. This way `try!` serves as an additional mark for developers that resolution can fail.