Prevent potential simultaneous access to cached values during cache reset

[jflan-dd]

I ran into an issue where one our mock dependencies is holding onto a type which accesses a different dependency during deinit.

When the cache is reset the mock dependency is released and the instance it's holding calls its deinit method, which causes it to attempt to access the dependency cache while the cache is still being written to, resulting in a simultaneous access exception.

In our instance this wouldn't come up in production both because we never reset the cache in production, but also because the live implementation of the dependency vends new instances of the reference type for every call rather than holding onto a shared instance internally.

The reportDependencyIssue change isn't required, but I didn't feel like adding a third instance of the complicated argument and dependencyDescription logic.

[mbrandonw]

Hi @jflan-dd, thanks for this! I can definitely confirm that your test crashes on main, and that with your change you avoid the crash and report an issue instead.

However, I am curious, considering that dependencies are long living, what is the use case for accessing a @Dependency in a deinit? It doesn't appear that it's possible for the deinit to be invoked when running on a device, and so is it only there for testing purposes?

but also because the live implementation of the dependency vends new instances of the reference type for every call rather than holding onto a shared instance internally.

Can you explain this a bit more? Every call of what?

[jflan-dd]

@mbrandonw We using a factory setup similar to this as a dependency:

protocol ViewControllerFactory {
  @MainActor
  func makeViewController() -> UIViewController
}

struct LiveViewControllerFactory: ViewControllerFactory {
  func makeViewController() -> UIViewController {
    CustomViewController()
  }
}

@MainActor
class MockViewControllerFactory: ViewControllerFactory {
  var mock = CustomViewController()
  
  func makeViewController() -> UIViewController { mock }
}

The view controller we're vending checks an experiment during it deinit similar to this:

final class CustomViewController: UIViewController {
  // ...
  
  deinit {
    @Dependency(\.experimentService) var experimentService

    if experimentService.isFeatureEnabled {
        // Do a custom action
    }
  }
}

In production the CustomViewController isn't an instance property of the ViewControllerFactory, but our mock conformance holds onto an instance of CustomViewController internally which results in the CustomViewController being release along with the ViewControllerFactory dependency when the cache is reset.

[mbrandonw]

Hi @jflan-dd, thanks for providing more context.

I must be missing something though. In your new code snippet, the thing with the deinit isn't a dependency itself (as in, it doesn't conform to DependencyKey). Is it possible to trigger this crash even when the class with the deinit is not a dependency? And if so, can you update your test case to show that situation?

[jflan-dd]

@mbrandonw I updated the test to more closely reflect the issue we're actually running into.

There is one annoying issue I haven't been able to resolve. This change prevents a crash, but the new failure gets attributed to the wrong test if it's not explicitly handled.

Since the cache reset happens in testCaseWillStart the access failure (formerly crash) happens during the start of the next test when the test observer resets the cache.

I haven't figured out a way to properly attribute the failure, but I could add to the failure message to direct the user to look at the previous test as the likely cause of the error.

[mbrandonw]

I haven't figured out a way to properly attribute the failure, but I could add to the failure message to direct the user to look at the previous test as the likely cause of the error.

I'm not sure that would be correct because multiple tests can run in parallel, especially in a Swift Testing world. Further, I believe recording an issue when outside of a test actually crashes Swift Testing when run in Xcode, and so there may still be a crash lurking in the shadows here.

Also, what is the fix if someone encounters this failure? The other reportIssues in this file describe what the user can do to fix the issue.