Skip to content

Commit

Permalink
Tests: avoid race-condition in leak detection (#2806)
Browse files Browse the repository at this point in the history
Fixes
https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/12839/workflows/2b1e24b9-428a-40ed-bcfd-a94d7591236d/jobs/89720/steps

I noticed when downloading the `.xcresult` that the tests had crashed
with this:
```
Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   libswiftCore.dylib            	       0x10bc64589 swift::runtime::AccessSet::insert(swift::runtime::Access*, void*, void*, swift::ExclusivityFlags) + 73
1   libswiftCore.dylib            	       0x10bc647e2 swift_beginAccess + 66
2   BackendIntegrationTests       	       0x13ab37c76 default argument 1 of Expectation.toEventually(_:timeout:pollInterval:description:) + 54
3   BackendIntegrationTests       	       0x13ab37b44 closure #1 in BaseBackendIntegrationTests.verifyPurchasesDoesNotLeak() + 276 (BaseBackendIntegrationTests.swift:167)
```

Turns out that because of the way it was written, normally `Nimble`
tries to print the instance during a failure:
```
BasePurchasesTests.swift:116: error: -[UnitTests.PurchasesConfiguringTests testSettingTheDelegateAfterInitializationSendsCachedCustomerInfo] : failed - Purchases has leaked
expected to eventually be nil, got <<RCPurchases: 0x125d13bf0>>
```

That crash points to a race condition inside Swift's runtime. To avoid
that, this changes the expectation so `Nimble` would only try to print
"expected true, got false", with our own provided description. Therefore
making sure that it doesn't try to access the instance while it's being
deallocated.
  • Loading branch information
NachoSoto authored Jul 13, 2023
1 parent 1c3bed0 commit e2ecdd0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ private extension BaseBackendIntegrationTests {
// - These run *before* `tearDown`.
// - They run in LIFO order.
self.addTeardownBlock { [weak purchases = Purchases.shared] in
expect(purchases).toEventually(beNil(), description: "Purchases has leaked")
// Note: this captures the boolean to avoid race conditions when Nimble tries
// to print `purchases` while it's being deallocated.
expect { purchases == nil }.toEventually(beTrue(), description: "Purchases has leaked")
}

self.addTeardownBlock {
Expand Down
14 changes: 8 additions & 6 deletions Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,14 @@ class BasePurchasesTests: TestCase {
Purchases.clearSingleton()
self.clearReferences()

expect(purchases)
.toEventually(beNil(), description: "Purchases has leaked")
expect(orchestrator)
.toEventually(beNil(), description: "PurchasesOrchestrator has leaked")
expect(deviceCache)
.toEventually(beNil(), description: "DeviceCache has leaked: \(self)")
// Note: this captures the boolean to avoid race conditions when Nimble tries
// to print the instances while they're being deallocated.
expect { purchases == nil }
.toEventually(beTrue(), description: "Purchases has leaked")
expect { orchestrator == nil }
.toEventually(beTrue(), description: "PurchasesOrchestrator has leaked")
expect { deviceCache == nil }
.toEventually(beTrue(), description: "DeviceCache has leaked")
}
}

Expand Down

0 comments on commit e2ecdd0

Please sign in to comment.