Skip to content
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

Fixed crash for collaborating containers #179

Merged
merged 5 commits into from
Dec 12, 2017

Conversation

trimmurrti
Copy link
Contributor

@trimmurrti trimmurrti commented Oct 30, 2017

This PR fixes a crash, that happens because of singleton state restoration in collaborating container that corrupts the singletons in the container to collaborate with.

The reproduction was found thanks to the fresh thoughts by @PaulTaykalo, as I was stuck on it.

The reproduction is outlined in testThatItCanHandleSeparateContainersAndTheirCollaboration in DipTests.

For some reason there appeared to be a crash under some really hard to meet conditions. It happened, when restoring state in defer in collaboratingResolve method.

The code below reproduces the crash:

class Manager {}
class AnotherManager {}

class Object {
  let manager: Manager?
  
  init(with manager: Manager?) {
    self.manager = manager
  }
}

class Owner {
  var manager: Manager?
}

extension DipTests {
  func testThatItCanHandleSeparateContainersAndTheirCollaboration() {
    let container = self.container
    
    let anotherContainer = DependencyContainer()
    anotherContainer.register { Object(with: anotherContainer) }
    
    container.collaborate(with: anotherContainer)
    
    container
      .register { Owner() }
      .resolvingProperties { $1.manager = try $0.resolve() }
    
    container.register(.singleton) { AnotherManager() }
    container.register(.singleton) { Manager() }
    
    let manager: Manager? = try? container.resolve()
    let another: AnotherManager? = try? container.resolve()
    var owner: Owner? = try? container.resolve(arguments: 1, "")
    
    let object: Object? = try? container.resolve()
    owner = try? container.resolve()
    
    let nonNilValues: [Any?] = [another, manager, owner, object, object?.manager]
    nonNilValues.forEach { XCTAssertNotNil($0) }
    
    XCTAssertTrue(
      owner?.manager
        .flatMap { value in
          manager.flatMap { $0 === value }
        }
        ?? false
    )
  }
}

FYI: If Object and its registration are changed to the code below, the crash disappears:

class Object {
  let manager: Manager?
  
  init(with manager: Manager?) {
    self.manager = manager
  }
}

anotherContainer.register { Object(with: try anotherContainer.resolve()) }

@ilyapuchka ilyapuchka merged commit a8dd47c into AliSoftware:develop Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants